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

FIX: Lint/RedundantDirGlobSort unset SafeAutoCorrect #10446

Merged
merged 1 commit into from Mar 9, 2022

Conversation

friendlyantz
Copy link
Contributor

@friendlyantz friendlyantz commented Mar 9, 2022

Why:

Lint/RedundantDirGlobSort Cop is not safe.

Removing .sort breaks on Ruby 3.0 with DirGlob
please refer this repo for demo of this issue


What:

  • update config/default.yml to set Lint/RedundantDirGlobSort as NOT asSafeAutoCorrect

No related GitHub issue found.


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.

@friendlyantz friendlyantz force-pushed the ap/fix-sort branch 2 times, most recently from dc1570c to 04d4ea8 Compare Mar 9, 2022
@friendlyantz friendlyantz changed the title Lint/RedundantDirGlobSort unset SafeAutoCorrect FIX: Lint/RedundantDirGlobSort unset SafeAutoCorrect Mar 9, 2022
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 9, 2022

This also has to be documented within the cop as in https://docs.rubocop.org/rubocop/1.25/cops_lint.html#safety

@@ -2013,6 +2013,7 @@ Lint/RedundantDirGlobSort:
Description: 'Checks for redundant `sort` method to `Dir.glob` and `Dir[]`.'
Enabled: pending
VersionAdded: '1.8'
Copy link
Member

@koic koic Mar 9, 2022

Choose a reason for hiding this comment

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

Can you insert VersionChanged: '<<next>>'?

Suggested change
VersionAdded: '1.8'
VersionAdded: '1.8'
VersionChanged: '<<next>>'

Copy link
Contributor Author

@friendlyantz friendlyantz Mar 9, 2022

Choose a reason for hiding this comment

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

Updated, thank you

@friendlyantz friendlyantz marked this pull request as ready for review Mar 9, 2022
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 9, 2022

I still don't see a "Safety" section in the cop docs after your updates.

@friendlyantz friendlyantz force-pushed the ap/fix-sort branch 2 times, most recently from 347a1a5 to 18c0016 Compare Mar 9, 2022
@friendlyantz
Copy link
Contributor Author

friendlyantz commented Mar 9, 2022

I still don't see a "Safety" section in the cop docs after your updates.

@bbatsov sorry, missed that, my first open-source contribution. Docs updated accordingly.
Let me know if I missed anything / put bad description

@koic
Copy link
Member

koic commented Mar 9, 2022

Please edit Lint/RedundantDirGlobSort cop's source code comment for document updates. The following may be helpful.
#10270

@friendlyantz
Copy link
Contributor Author

friendlyantz commented Mar 9, 2022

@koic thank you, done 🙏🏻

@bbatsov bbatsov merged commit 857e682 into rubocop:master Mar 9, 2022
34 checks passed
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 9, 2022

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.

None yet

3 participants