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.

Sign up for GitHub

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

Jump to bottom

Add new cop Minitest/TestFileName #217

Merged
merged 1 commit into from
Jan 10, 2023
Merged

Add new cop Minitest/TestFileName #217

merged 1 commit into from
Jan 10, 2023

Conversation

tejasbubane
Copy link
Contributor

Closes #213

Before submitting the PR make sure the following are checked:

  • 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.

fatkodima
fatkodima reviewed Dec 30, 2022
View reviewed changes
Copy link
Contributor

@fatkodima fatkodima left a comment

Choose a reason for hiding this comment

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

❤️

lib/rubocop/cop/minitest/file_name.rb Outdated Show resolved Hide resolved
lib/rubocop/cop/minitest/file_name.rb Outdated Show resolved Hide resolved
config/default.yml Outdated Show resolved Hide resolved
fatkodima
fatkodima reviewed Dec 30, 2022
View reviewed changes
Copy link
Contributor

@fatkodima fatkodima left a comment

Choose a reason for hiding this comment

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

A small suggestion, in comment. Overall, lgtm 💪 Thank you!

lib/rubocop/cop/minitest/file_name.rb Outdated Show resolved Hide resolved
@fatkodima
Copy link
Contributor

Why did you decided to move from on_class to on_new_investigation? 🤔 Not a big deal, but the previous one looked simpler, imo.
And I would probably change the name from FileName to TestFileName, similar to Minitest/TestMethodName cop.

@tejasbubane
Copy link
Contributor Author

tejasbubane commented Jan 1, 2023 •
edited

Why did you decided to move from on_class to on_new_investigation?

@fatkodima Because if a test file has multiple classes, on_class will register error for each class.

@tejasbubane tejasbubane changed the title Add new cop Minitest/FileName Add new cop Minitest/TestFileName Jan 1, 2023
@tejasbubane tejasbubane force-pushed the fix-213 branch 3 times, most recently from 527be8f to 21cf39b Compare January 1, 2023 11:37
@fatkodima
Copy link
Contributor

Can't we call add_global_offense in a on_class?

@tejasbubane
Copy link
Contributor Author

Can't we call add_global_offense in a on_class?

I guess we could, but the cop will be invoked on every class definition which is not needed.

@fatkodima
Copy link
Contributor

This shouldn't be a large problem, but yeah, makes sense. Lgtm then 👍

@koic
Copy link
Member

koic commented Jan 8, 2023

@tejasbubane ping.

@tejasbubane tejasbubane requested review from koic and removed request for fatkodima January 9, 2023 16:14
@koic koic merged commit ca0c1a3 into rubocop:master Jan 10, 2023
@koic
Copy link
Member

koic commented Jan 10, 2023

Thanks!

@tejasbubane tejasbubane deleted the fix-213 branch January 10, 2023 09:50
koic added a commit to koic/rubocop that referenced this pull request Feb 22, 2023
@koic
Add new InternalAffairs/ProcessedSourceBufferName cop
0b95014 
Follow up rubocop/rubocop-minitest#217 (comment).

This PR adds new `InternalAffairs/ProcessedSourceBufferName` cop.
It enforces the use of `processed_source.file_path` instead of `processed_source.buffer.name`.

```ruby
# bad
processed_source.buffer.name

# good
processed_source.file_path
```
@koic koic mentioned this pull request Feb 22, 2023
Merged
8 tasks
bbatsov pushed a commit to rubocop/rubocop that referenced this pull request Feb 22, 2023
@koic @bbatsov
Add new InternalAffairs/ProcessedSourceBufferName cop
d16167e 
Follow up rubocop/rubocop-minitest#217 (comment).

This PR adds new `InternalAffairs/ProcessedSourceBufferName` cop.
It enforces the use of `processed_source.file_path` instead of `processed_source.buffer.name`.

```ruby
# bad
processed_source.buffer.name

# good
processed_source.file_path
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andyw8 andyw8 andyw8 left review comments

@fatkodima fatkodima fatkodima left review comments

@koic koic Awaiting requested review from koic

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New cop: Minitest/TestFileName
4 participants
@tejasbubane @fatkodima @koic @andyw8