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

Naming/FileName: make CheckDefinitionPathHierarchy roots configureable #10224

Merged
merged 1 commit into from Nov 15, 2021

Conversation

grosser
Copy link
Contributor

@grosser grosser commented Oct 29, 2021

fixes #10220

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • 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.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

confirmed that it fixes the issue:

parts/monitors/foo/bar.rb:1:1: C: Naming/FileName: bar.rb should define a class or module called Monitors::Foo::Bar. (https://rubystyle.guide#snake-case-files)
module Monitors
^

@grosser
Copy link
Contributor Author

grosser commented Nov 5, 2021

@dvandersluis could you take a look at this PR plz :)

@@ -381,9 +381,13 @@ anything/using_snake_case.rake
| `true`
| Boolean

| CheckDefinitionPathHierarchyRoots
Copy link
Member

Choose a reason for hiding this comment

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

The cop doc files are generated automatically so should not be included here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how are they generated ? ... on merge or I have to run something ?

Copy link
Member

Choose a reason for hiding this comment

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

They’re generated as part of the release process.

@dvandersluis
Copy link
Member

Can you please rebase? The specs were changed in #10223 (although magically this doesn't seem to have added a conflict somehow!)

Also, please add tests for CheckDefinitionPathHierarchyRoots specifically.

@grosser
Copy link
Contributor Author

grosser commented Nov 15, 2021

both the "file is under roots" and "file is not under roots" cases are already covered and the test uses the new configuration option ... but added a new test anyway
removed the doc addition

@grosser
Copy link
Contributor Author

grosser commented Nov 15, 2021

good to go ?

@dvandersluis dvandersluis merged commit e152d25 into rubocop:master Nov 15, 2021
@dvandersluis
Copy link
Member

Thanks @grosser !

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