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
Suggestion: rename --ci option to a context-neutral name #1
Comments
Another option would be a design where the following behaviors are separated:
I think that erroring out if there are new issues could be the default, so that the Something like: # Default: errors out on new issues, updates lock file
# with positive changes if there are no new issues
rubocop-gradual
# For CI, might be useful to disable updating so that
# it doesn't modify files
rubocop-gradual --no-update
# Forcing updates, should never error out
# (except for errors in rubocop-gradual, IO errors, etc.)
rubocop-gradual --force-update (This also suggests renaming In that model, I’m a bit unclear if |
Hey, @fvsch thanks for the issue! Overall, I agree to your suggestions. Love the
I think it should update only when there are no errors because partial update might lead to overuse of force updating:
|
Still brainstorming a bit. (Sorry for blowing up the scope of this issue, but since it's early days for this tool it can be fun to geek out about the API design ^^) Proposed design# Default: runs rubocop, compares its output to the rubocop-gradual lock file,
# and shows a report with improvements (fixed issues) and regressions (new issues).
# Does NOT modify the lock file.
rubocop-gradual
# Checks that linting output strictly matches the rubocop-gradual lock file.
# Exits with an error code for any change that is unaccounted for:
# regressions *and* improvements.
# We recommend using this option in CI.
rubocop-gradual --check
# Update the lock file to account for improvements (if any).
# If there is a regression (new issue): exits with an error code,
# and does not touch the lock file.
rubocop-gradual --update
# Update the lock file to match the current linting output, including regressions.
# Only use exceptionally, and review changes to the lock file to make sure you are
# not adding unwanted regressions.
rubocop-gradual --force-update Some highlights:
Exit codesI think exit codes would work like this:
(Question: should With those exit codes:
Question: loose checking in CI?My only concern with this design is whether users would like to do more permissive checking in CI. We use Betterer at StackBlitz and it's sometimes frustrating in our development workflow because of its strict checking. Scenario:
If we used more permissive checking, where only new issues (and probably existing issues that got moved around) would fail checks in CI, the results would be:
I’m not sure what's the best trade-off here. Should there be no loose checking option because it leads to bad results over time? Or should there be an additional option that is explicitly for loose checking, say (Note that in this proposal, both |
After a bit more discussion, the idea is to keep So, keeping rubocop-gradual as a command with side-effects, intended for the main development workflow:
So I would consider this issue solved if the I’ll open another issue about the |
The
--ci
option is ambiguous:If the functionality is "check code against the lock file and error out if there is any new linting output", maybe a name like
--check
might be a better fit?Edit: other brainstormed names:
--test
,--strict
.The text was updated successfully, but these errors were encountered: