ruff icon indicating copy to clipboard operation
ruff copied to clipboard

Remove deprecated configuration '--show-source`

Open tibor-reiss opened this issue 1 year ago • 6 comments

Fixes parts of https://github.com/astral-sh/ruff/issues/7650:

Extracted from this PR 1. removes the deprecated configuration 'format'. It was deprecated in https://github.com/astral-sh/ruff/pull/8203 in favor of 'output-format'. Update: Was merged in https://github.com/astral-sh/ruff/pull/10170 Tests: - `cargo run -p ruff -- --explain RET505` works - `cargo run -p ruff -- --explain RET505 --output-format json` works - `cargo run -p ruff -- --explain RET505 --format json` does not work anymore
  1. removes deprecated configurations 'show-source' and 'no-show-source'. Tests:
  • cargo run -p ruff -- --show-source does not work anymore
  • cargo run -p ruff -- --no-show-source does not work anymore

resolves: #7349

tibor-reiss avatar Feb 04 '24 19:02 tibor-reiss

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

github-actions[bot] avatar Feb 04 '24 19:02 github-actions[bot]

Thanks for your contribution! We won't be able to release this until v0.3.0 so it's going to be a bit before we can merge

zanieb avatar Feb 04 '24 21:02 zanieb

Hi @zanieb, since there are more of these deprecations in the codebase, do you mind if I keep adding commits to this PR, or shall I open a separate one for each of them?

tibor-reiss avatar Feb 05 '24 07:02 tibor-reiss

Hey @tibor-reiss — I'm unsure. Ideally these things would be in separate pull requests for our changelog and review but we're talking pretty far out here and it could be a significant amount to keep your changes up to date with main. In general, I'd recommend pursuing issues that aren't breaking changes since they aren't timing sensitive.

Another thing we can do is have deprecated options error in preview mode first but retain a warning in stable.

zanieb avatar Feb 08 '24 19:02 zanieb

Hi @zanieb, I understand. I added the 2nd commit just before your answer - talk about timing. And as you predicted, merge conflict. Since the timeline is unclear, and this is anyways a low-prio change, I would recommend gathering the commits here and not polluting the board with more PRs. Once we get closer, it's a minimal change to pull out the commits into separate PRs so you can ping me about your preferences about this change. What do you think?

tibor-reiss avatar Feb 08 '24 20:02 tibor-reiss

@tibor-reiss I extracted the first breaking change into its own PR and ship it as part of 0.3. We want to wait a little longer with the other options because it isn't that long ago that we've deprecated them.

MichaReiser avatar Feb 29 '24 13:02 MichaReiser