-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Fix breeze setup autocomplete references
#58898
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
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
|
To follow up on this in the future, do you think it's worth it to cover this and similar paths of such setup scripts (and their examples in the docs) with some tests, just in case? I could file an issue to track. |
potiuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice.
|
Interesting. Main reason why it was not working was that recently - in August - #54258 when I removed some very, very old deprecated commands (
Actually this is one of the very few scripts not covered by CI - and I take it almost as a pride point to test all those scripts. This one has not been tested, because it was mostly a helper. But ABSOLUTELY .... Feel free to add it to our CI :) |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
|
Thanks my friend BTW. Glad to see you here :) |
* Fix `setup autocomplete` invocation * Correct autocomplete command tip (cherry picked from commit c116edc) Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
* Fix `setup autocomplete` invocation * Correct autocomplete command tip (cherry picked from commit c116edc) Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
🫡 |
* Fix `setup autocomplete` invocation * Correct autocomplete command tip (cherry picked from commit c116edc) Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
* Fix `setup autocomplete` invocation * Correct autocomplete command tip
* Fix `setup autocomplete` invocation * Correct autocomplete command tip
* Fix `setup autocomplete` invocation * Correct autocomplete command tip
The
./scripts/tools/setup_breezescript fails to install autocompletions. To reproduce:Details
In #25449, the
setup-autocompletecommand becamesetup autocomplete, but${BREEZE_BINARY} setup-autocompletein the setup script (which used to be./breezeback then) wasn't fixed up -- 31fb5ff#diff-7e92c11499fd4106a899eeb457db8277bd6f09dd29e683f33ae704e12fa29caeR73. A trickier one :)I've fixed the old-fashion command invocation, and I've corrected the remaining stale references to
setup-autocompletein the docs.You shouldn't find
setup-autocompletenow anywhere but in CSS class names.cc @potiuk (hi! 😁)
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.