📌 Loosen RuboCop dep requirements#63
Conversation
|
If this is a desired change, should I work on fixing the test suite in GA? |
|
⭕ What is the release cadence for betterlint? If the gem restrictions remain locked down to patch level as they are, and the gem is not released frequently, I'll have to stop using it in my projects, because the rest of the Ruby world is moving forward and this gem is now a blocker on several fronts, but most annoyingly on P.S. the voluminous deprecation warnings are also a significant bother: #66 |
…itation. - https://github.com/Betterment/betterlint/actions/runs/12437356175/job/34727086761?pr=63 - ruby 3.3.6 (2024-11-05 revision 75015d4c1f) [x86_64-linux]
|
@smudge Needs another workflow run approval? |
| - ruby: "truffleruby-head" | ||
| appraisal: "rails-8-0" | ||
| taskname: "spec" | ||
| gemfile: "appraisal_root" | ||
| rubygems: latest | ||
| bundler: latest |
There was a problem hiding this comment.
We've been operating largely as an MRI-only shop. Not rejecting this outright, but I'd be interested in the motivation for including it in a separate heads matrix.
For now, I'd be inclined to defer this to a separate PR/discussion, and consolidate MRI head down into tests.yml (if building against head is even an active goal here).
There was a problem hiding this comment.
Building against head is just a canary, an early warning about upcoming technical debt when you discover things will potentially break in the next Ruby release, once it is released.
IMO it is most useful as a discrete workflow because it should be allowed to fail, and while GitHub doesn't have a proper "allow failures" feature, they should, and they will.
There was a problem hiding this comment.
I'll remove the truffleruby-head. I add it to my open source gems whenever I can because I like to support it.
There was a problem hiding this comment.
By the way, this churn getting the CI to work is because your setup is different from the type I usually run (I always add development dependencies to the gemspec, in violation of the default RuboCop rule, because it simplifies things for me...) I'll have it working shortly.
Error: The `RSpec/Capybara/FeatureMethods` cop has been removed since this cop has migrated to `RSpec/Dialect`. Please use `RSpec/Dialect` instead. (obsolete configuration found in config/default.yml, please update it) `RSpec/Capybara/*` has been extracted to the `rubocop-capybara` gem. (obsolete configuration found in config/default.yml, please update it) Error: unrecognized cop or department FactoryBot/AssociationStyle found in config/default.yml unrecognized cop or department FactoryBot/ConsistentParenthesesStyle found in config/default.yml unrecognized cop or department FactoryBot/SyntaxMethods found in config/default.yml
| s.add_dependency "rubocop-capybara", "~> 2.21" | ||
| s.add_dependency "rubocop-factory_bot", "~> 2.26" |
There was a problem hiding this comment.
I'd be interested in adding rubocop-capybara and rubocop-factory_bot in an independent PR as well. I'm not sure if they were intentionally excluded, but baking them into our in-house preferences would merit at least a separate set of sign-offs, and I'd like to keep the path clear for you to merge this PR sooner.
There was a problem hiding this comment.
There are rules in the default config which make it impossible to run without them. It seems they are currently accidental undeclared dependencies. I only added them because I was forced to.
Errors were:
Error: The `RSpec/Capybara/FeatureMethods` cop has been removed since this cop has migrated to `RSpec/Dialect`. Please use `RSpec/Dialect` instead.
(obsolete configuration found in config/default.yml, please update it)
`RSpec/Capybara/*` has been extracted to the `rubocop-capybara` gem.
(obsolete configuration found in config/default.yml, please update it)
and
Error: unrecognized cop or department FactoryBot/AssociationStyle found in config/default.yml
unrecognized cop or department FactoryBot/ConsistentParenthesesStyle found in config/default.yml
unrecognized cop or department FactoryBot/SyntaxMethods found in config/default.yml
AFACT, if they are removed, then the rules in the default config must be removed as well.
There was a problem hiding this comment.
Actually, I did remove the Capybara rule it complained about, because the replacement suggestion didn't make much sense to me, saying both RSpec/Dialect, and rubocop-capybara, which don't sound related.
But since it had been there it was clear that rubocop-capybara was intended to be a dependency. I don't mind removing it, as I don't think there are any rules remaining that relate to it.
The factory-bot rules are still in the config though, so I think the gem is still required.
There was a problem hiding this comment.
The GitHub Actions workflows are now passing! This is ready for review.
I removed rubocop-capybara, but not rubocop-factory_bot, since the build won't pass without it (see comments above).
There was a problem hiding this comment.
Ping @smudge , I'm working on recraeting this PR after rebasing on top of your latest changes, but I'd like guidance on the above issue of rubocop-factory_bot being a legitimate undeclared dependency. It seems like the gem can't work without it.
|
I'm closing this one in favor of #73 |
3 participants
Replaced by #73
It is great to use this gem in tandem with
standard, but when they get out of sync it is painful. It isn't clear why the patch restriction is useful (aside fromrubocopnot following semantic versioning, while claiming it does). In practice it is not useful to me, as I constrainrubocopseparately, so I've removed it. I don't expect this to get merged, but would like to understand the thinking behind the constraints. Maybe it would be useful to add them as comments to the gemspec.