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 deprecated configuration '--show-source` #9814

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tibor-reiss
Copy link
Contributor

@tibor-reiss tibor-reiss commented Feb 4, 2024

Fixes parts of #7650:

Extracted from this PR 1. removes the deprecated configuration 'format'. It was deprecated in https://github.com//pull/8203 in favor of 'output-format'. Update: Was merged in https://github.com//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 tibor-reiss force-pushed the remove-deprecated-configuration branch from 12d2d69 to 24e6538 Compare February 4, 2024 19:05
Copy link
Contributor

github-actions bot commented Feb 4, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@zanieb
Copy link
Member

zanieb commented Feb 4, 2024

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 zanieb added this to the v0.3.0 milestone Feb 4, 2024
@zanieb zanieb added the breaking Breaking API change label Feb 5, 2024
@tibor-reiss
Copy link
Contributor Author

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 tibor-reiss force-pushed the remove-deprecated-configuration branch from e5a7e31 to ed2866c Compare February 8, 2024 19:37
@zanieb
Copy link
Member

zanieb commented Feb 8, 2024

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.

@tibor-reiss
Copy link
Contributor Author

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?

@MichaReiser MichaReiser removed this from the v0.3.0 milestone Feb 29, 2024
@MichaReiser
Copy link
Member

@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.

@tibor-reiss tibor-reiss force-pushed the remove-deprecated-configuration branch from ed2866c to 9d99534 Compare March 1, 2024 21:20
@MichaReiser MichaReiser changed the title Remove deprecated configuration 'format' Remove deprecated configuration '--show-source` Mar 4, 2024
@MichaReiser MichaReiser added this to the v0.4.0 milestone Mar 4, 2024
@dhruvmanila dhruvmanila modified the milestones: v0.4.0, v0.5.0 Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking API change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI: Show source context by default in ruff check
4 participants