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

Remove support for providing output format via format option #7984

Merged
merged 5 commits into from
Oct 16, 2023

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Oct 16, 2023

See the provided breaking changes note for details.

Removes support for the deprecated --formatoption in the ruff check CLI, format inference as output-format in the configuration file, and the RUFF_FORMAT environment variable.

The error message for use of format in the configuration file could be better, but would require some awkward serde wrappers and it seems hard to present the correct schema to the user still.

@zanieb zanieb added breaking Breaking API change configuration Related to settings and configuration labels Oct 16, 2023
zanieb added a commit that referenced this pull request Oct 16, 2023
@zanieb zanieb marked this pull request as ready for review October 16, 2023 17:41
warning: The option `format` has been deprecated to avoid ambiguity with Ruff's upcoming formatter. Use `output-format` instead.
"###);
insta::with_settings!({filters => vec![
(&*regex::escape(ruff_toml.to_str().unwrap()), "[RUFF-TOML-PATH]"),
Copy link
Member

Choose a reason for hiding this comment

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

Huzzah

@github-actions
Copy link
Contributor

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

@zanieb zanieb merged commit 523f542 into main Oct 16, 2023
16 checks passed
@zanieb zanieb deleted the zanie/remove-format branch October 16, 2023 18:06
@acdha
Copy link

acdha commented Oct 16, 2023

This broke everyone's CI without warning. It's not that hard to fix but at the very least it would have been nice to have advanced notice and perhaps a more meaningful error message.

acdha added a commit to django-haystack/pysolr that referenced this pull request Oct 16, 2023
@zanieb
Copy link
Member Author

zanieb commented Oct 16, 2023

Hey @acdha I'm sorry to hear this broke your CI.

Use of this displayed warnings since v0.0.291 and the changelog for that release had a bolded notice. Additionally, we grouped this breaking change with an increase in a different version number per our versioning policy.

I'm not sure what else we could have done here, except to have a longer deprecation period — which we will generally strive for, but in this case the error messages when the format configuration option was used incorrectly were confusing many of our users and challenging to improve without dropping support as we did here.

acdha added a commit to django-haystack/pysolr that referenced this pull request Oct 16, 2023
acdha added a commit to django-haystack/pysolr that referenced this pull request Oct 16, 2023
@acdha
Copy link

acdha commented Oct 16, 2023

Hey @acdha I'm sorry to hear this broke your CI.

Use of this displayed warnings since v0.0.291 and the changelog for that release had a bolded notice. Additionally, we grouped this breaking change with an increase in a different version number per our versioning policy.

I'm not sure what else we could have done here, except to have a longer deprecation period — which we will generally strive for, but in this case the error messages when the format configuration option was used incorrectly were confusing many of our users and challenging to improve without dropping support as we did here.

Yeah, I appreciate that there isn't a great answer here - a longer period than a couple of weeks might have helped but given that most of the time it's running in CI where people rarely look at passing builds I think the part which would have helped more would have been having an error message specifically noting that this option was renamed. Long-term, I'm sure the best solution would be an official GitHub Action which you could update at the same time as any future API changes.

justinchuby added a commit to justinchuby/lintrunner-adapters that referenced this pull request Oct 16, 2023
The `--format` option has been renamed to `--output-format` in astral-sh/ruff#7984
justinchuby added a commit to justinchuby/lintrunner-adapters that referenced this pull request Oct 16, 2023
The `--format` option has been renamed to `--output-format` in
astral-sh/ruff#7984

This is a breaking change and we require ruff>=0.0.291 to be compatible.
kstrauser added a commit to kstrauser/python-lsp-ruff that referenced this pull request Oct 17, 2023
In astral-sh/ruff#7984, ruff removed support for
the "--output" command line option, and now requires "--output-format"
instead.
fannheyward added a commit to fannheyward/coc-pyright that referenced this pull request Oct 17, 2023
@MichaReiser
Copy link
Member

This broke everyone's CI without warning. It's not that hard to fix but at the very least it would have been nice to have advanced notice and perhaps a more meaningful error message.

I'm sorry about that. What's your setup? Do you always install the latest Ruff version on CI? If so, I would recommend you to pin your ruff version and bump it periodically.

jhossbach pushed a commit to python-lsp/python-lsp-ruff that referenced this pull request Oct 19, 2023
In astral-sh/ruff#7984, ruff removed support for
the "--output" command line option, and now requires "--output-format"
instead.
charliermarsh pushed a commit that referenced this pull request Oct 28, 2023
…tion warnings (#8203)

## Summary

Since `--format` was changed to `--output-format` for `check`, it feels
like it makes sense for the same to work for the auxiliary commands.

This 

* adds the same deprecation warning that used to be a thing in #7514
(and un-became a thing in #7984)

Fixes #7990.

## Test Plan

* `cargo run --bin=ruff -- rule --all --output-format=json` works
* `cargo run --bin=ruff -- rule --format=json` works with warnings
skwashd added a commit to proactiveops/picofun that referenced this pull request Nov 16, 2023
skwashd added a commit to proactiveops/picofun that referenced this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking API change configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants