Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Document.descendants and.subclasses advice#356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
alexanderadam wants to merge1 commit intogithub:main
base:main
Choose a base branch
Loading
fromalexanderadam:doc/document_descendants_advice

Conversation

@alexanderadam
Copy link
Contributor

@alexanderadamalexanderadam commentedOct 21, 2025
edited
Loading

A wise man, let's call him Ben,once said:

There are two reasons why this is an unreliable solution:

  • it doesn't know about things that have yet to be autoloaded

  • it's non-deterministic with regards to Garbage Collection of classes. If you useClass.descendants in Test, where there is a pattern to dynamically define classes, GC is unpredictable for when those classes are cleaned up and removed by the GC.

And I totally trust him on that because he's making a… GoodJob 🥁

This is generally unwanted to be documented in Rails.
But I also understand Ben's reasoning.

Now the question is: do you want to have it here?
Shall I add the cops (1,2) here? 🤔

PS: I'm looking for a new adventure in case anybody is looking to hire or work with a Ruby/Rails/Crystal dev

@alexanderadamalexanderadam requested a review froma team as acode ownerOctober 21, 2025 18:00
CopilotAI review requested due to automatic review settingsOctober 21, 2025 18:00
Copy link

CopilotAI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Pull Request Overview

This PR adds guidance to the styleguide discouraging the use ofClass#descendants due to reliability issues. The addition documents thatClass#descendants is non-deterministic because it doesn't account for classes that haven't been autoloaded yet and can behave inconsistently with garbage collection, particularly in tests with dynamically defined classes.

  • Documents the unreliability ofClass#descendants with autoloading and garbage collection
  • Provides a code example demonstrating the problematic pattern

Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.

STYLEGUIDE.md Outdated
Comment on lines 1102 to 1104
Avoid using`Class#descendants` as it is unreliable.
It doesn't know about classes that haven't been autoloaded yet, and it's non-deterministic with regards to garbage collection of classes.
In tests that dynamically define classes, GC timing can mean`Class#descendants` may or may not include those dynamically defined classes.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

[nitpick] The documentation should provide an alternative approach or workaround for scenarios where developers might need to track subclasses. Without suggesting alternatives, developers may be unclear on what to use instead ofClass#descendants.

Copilot uses AI. Check for mistakes.
@alexanderadamalexanderadamforce-pushed thedoc/document_descendants_advice branch from0740e10 to0c4848bCompareOctober 27, 2025 21:32
@alexanderadamalexanderadam changed the titleDocumentClass.descendants adviceDocument.descendants and.subclasses adviceOct 27, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

Copilot code reviewCopilotCopilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@alexanderadam

[8]ページ先頭

©2009-2025 Movatter.jp