Skip to content

Conversation

@r7kamura
Copy link
Contributor

@r7kamura r7kamura commented Nov 18, 2025

A change related to keywords was introduced here:

At that time, the implementation was adjusted so that the following code would not be treated as an offense, but shouldn’t this actually be considered an offense?

# EnforcedStyle: implicit

# bad
association :foo, :alias

# good
foo factory: %i[foo alias]

In addition to changing the implementation, I made a few improvements:

  1. Converted KEYWORDS from an Array to a Set to improve performance.
  2. Updated the code so that only correctable explicit associations are treated as offenses, making the intention clearer.

Before submitting the PR make sure the following are checked:

  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Updated documentation.
  • Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
  • The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

If you have created a new cop:

  • Added the new cop to config/default.yml.
  • The cop is configured as Enabled: pending in config/default.yml.
  • The cop documents examples of good and bad code.
  • The tests assert both that bad code is reported and that good code is not reported.
  • Set VersionAdded: "<<next>>" in default/config.yml.

If you have modified an existing cop's configuration options:

  • Set VersionChanged: "<<next>>" in config/default.yml.

@r7kamura r7kamura requested a review from a team as a code owner November 18, 2025 04:53
@pirj
Copy link
Member

pirj commented Nov 23, 2025

Will this combine with the other two? #167 #169
Maybe put them all in a single PR to avoid merges? I personally don't mind reviewing bigger PRs, especially when they address neighbouring issues.

@r7kamura
Copy link
Contributor Author

r7kamura commented Nov 23, 2025

I don't think these should be combined into a single pull request.

While it's true that they modify nearby lines of the same cop, increasing the likelihood of conflicts, these three changes fundamentally address different issues and each involves a modification to the cop's behavior.

There's a possibility that users may be affected by each change, and they would likely check this in CHANGELOG.md. In that case, it would be preferable for them to be able to see only the small pull requests relevant to them.

If any conflicts arise, I will resolve them as needed, and I don't mind doing that amount of work at all.

(Even so, if the opinion from the maintainer's side is that merging such finely divided pull requests is too difficult, I will consolidate them, so please let me know.)

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.

2 participants