Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add autocorrection for Rails/ReflectionClassName #299

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

tejasbubane
Copy link
Contributor

Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • If this is a new cop, consider making a corresponding update to the Rails Style Guide.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@tejasbubane
Copy link
Contributor Author

tejasbubane commented Jul 21, 2020

Waiting for #302 to be merged to fix the CI. Fixed

@tejasbubane tejasbubane force-pushed the reflection-class-name-autocorrect branch 2 times, most recently from d17dbb7 to dfc20d7 Compare July 22, 2020 06:44
@koic
Copy link
Member

koic commented Jun 6, 2022

@tejasbubane ping.

@tejasbubane tejasbubane force-pushed the reflection-class-name-autocorrect branch from dfc20d7 to fc90afb Compare January 4, 2023 22:48
@tejasbubane
Copy link
Contributor Author

tejasbubane commented Jan 4, 2023

@koic Sorry for the delay on this one, I've made the changes to autocorrect only for constants or .name or .to_s.

@tejasbubane tejasbubane force-pushed the reflection-class-name-autocorrect branch from fc90afb to 504f14d Compare January 4, 2023 23:11
@tejasbubane tejasbubane requested review from koic and pirj and removed request for koic January 4, 2023 23:11
@koic
Copy link
Member

koic commented Apr 3, 2023

@tejasbubane Can you rebase with the latest master branch?

@tejasbubane tejasbubane force-pushed the reflection-class-name-autocorrect branch from 504f14d to 52578e2 Compare April 3, 2023 10:16
@tejasbubane
Copy link
Contributor Author

@koic Rebased.

@tejasbubane tejasbubane requested a review from koic April 3, 2023 10:19
@koic koic merged commit 2a648b0 into rubocop:master Apr 3, 2023
@koic
Copy link
Member

koic commented Apr 3, 2023

Thanks!

@tejasbubane tejasbubane deleted the reflection-class-name-autocorrect branch April 3, 2023 23:06
@alex-tan
Copy link

alex-tan commented Nov 6, 2023

Is there an advantage to using this rule or is this just a stylistic choice? The advantage I can see to using reflection is that if a class name changes and your relation relies on it, it will be obvious very quickly as your app will fail to start if you have failed to update references to the underlying class.

@pirj
Copy link
Member

pirj commented Nov 6, 2023

It will fail both ways, won’t it?
I guess to save on cascade class autoloading.

@alex-tan
Copy link

alex-tan commented Nov 7, 2023

Ah I see. If there's a circular dependency.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants