Skip to content

Conversation

@Lee-W
Copy link
Contributor

@Lee-W Lee-W commented Apr 23, 2025

Summary

This is not yet fixing anything as the names are not changed, but it lays down the foundation for fixing.

Test Plan

the existing test fixture should already cover this change

@Lee-W Lee-W changed the title Auto import air312 [airflow] apply Replacement::AutoImport to AIR312 Apr 23, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 23, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@Lee-W Lee-W force-pushed the auto-import-AIR312 branch from 446ba90 to 95706cb Compare April 28, 2025 07:26
@Lee-W Lee-W marked this pull request as ready for review April 29, 2025 01:19
@Lee-W
Copy link
Contributor Author

Lee-W commented Apr 30, 2025

Hey @ntBre , this is also ready to be reviewed 🙂

btw if we would like to mark AIR3XX as stable, what would be the requirement? are we allow to gradually improve the rules afterward?

@ntBre
Copy link
Contributor

ntBre commented Apr 30, 2025

btw if we would like to mark AIR3XX as stable, what would be the requirement? are we allow to gradually improve the rules afterward?

Going by the rule stabilization docs, the main normal requirement is that a rule has been in preview for one minor release cycle. So since these are going into 0.11.x, I think we usually wouldn't consider stabilizing them until 0.13.0, or at least 0.12.0.

That page also says:

Stable rule behaviors are not changed significantly in patch versions

So we wouldn't want to change them drastically after stabilization, but if you mean something more like adding a few new deprecated imports, I think that would be okay.

These rules feel like a bit of an exception to the general guidelines to me, so we could possibly consider stabilizing them sooner, but I'd defer to @MichaReiser or @dhruvmanila on that decision.

Were you just hoping that users could access these without enabling preview mode?

@ntBre ntBre added rule Implementing or modifying a lint rule preview Related to preview mode features labels Apr 30, 2025
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@ntBre ntBre merged commit 6e765b4 into astral-sh:main Apr 30, 2025
33 checks passed
@ntBre
Copy link
Contributor

ntBre commented Apr 30, 2025

I noticed this a bit late, but is the PR title supposed to say AIR302? I'll try to remember to update this in the changelog tomorrow if so.

@dhruvmanila
Copy link
Member

I noticed this a bit late, but is the PR title supposed to say AIR302? I'll try to remember to update this in the changelog tomorrow if so.

You can update it right now as, I think, rooster will fetch the latest PR title :)

@ntBre
Copy link
Contributor

ntBre commented Apr 30, 2025

You can update it right now as, I think, rooster will fetch the latest PR title :)

Ahh good to know, I assumed that it took the commit message. Thanks!

@ntBre ntBre changed the title [airflow] apply Replacement::AutoImport to AIR312 [airflow] apply Replacement::AutoImport to AIR302 Apr 30, 2025
@dhruvmanila
Copy link
Member

Everything that @ntBre said is what we usually follow. In this case we could possibly stabilize it in the next minor release but that isn't planned in the near future (possibly in June). Do you think there would be any issue in recommending users to use the --preview flag when migrating? It can be clubbed with --select flag to only select certain rules so that other preview rules are not selected. I've opened #17749 to track this.

dcreager added a commit that referenced this pull request May 1, 2025
* main:
  [red-knot] Preliminary `NamedTuple` support (#17738)
  [red-knot] Add tests for classes that have incompatible `__new__` and `__init__` methods (#17747)
  Update dependency vite to v6.2.7 (#17746)
  [red-knot] Update call binding to return all matching overloads (#17618)
  [`airflow`] apply Replacement::AutoImport to `AIR312` (#17570)
  [`ruff`] Add fix safety section (`RUF028`) (#17722)
  [syntax-errors] Detect single starred expression assignment `x = *y` (#17624)
  py-fuzzer: fix minimization logic when `--only-new-bugs` is passed (#17739)
  Fix example syntax for pydocstyle ignore_var_parameters option (#17740)
  [red-knot] Update salsa to prevent panic in custom panic-handler (#17742)
  [red-knot] Ban direct instantiation of generic protocols as well as non-generic ones (#17741)
  [red-knot] Lookup of `__new__` (#17733)
  [red-knot] Check decorator consistency on overloads (#17684)
  [`flake8-use-pathlib`] Avoid suggesting `Path.iterdir()` for `os.listdir` with file descriptor (`PTH208`) (#17715)
  [red-knot] Check overloads without an implementation (#17681)
  Expand Semantic Syntax Coverage (#17725)
  [red-knot] Check for invalid overload usages (#17609)
@Lee-W
Copy link
Contributor Author

Lee-W commented May 2, 2025

Everything that @ntBre said is what we usually follow. In this case we could possibly stabilize it in the next minor release but that isn't planned in the near future (possibly in June). Do you think there would be any issue in recommending users to use the --preview flag when migrating? It can be clubbed with --select flag to only select certain rules so that other preview rules are not selected. I've opened #17749 to track this.

Got it. The only concern we have is the --preview flag, but it doesn't sound like a major concern. Thanks for letting us know! I'll bring the message back to the community!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants