Skip to content

Comments

Don't reuse MY_DIR in breeze to mean different folder from ci/_utils.sh#9098

Merged
potiuk merged 1 commit intoapache:masterfrom
astronomer:breeze-var-name
Jun 2, 2020
Merged

Don't reuse MY_DIR in breeze to mean different folder from ci/_utils.sh#9098
potiuk merged 1 commit intoapache:masterfrom
astronomer:breeze-var-name

Conversation

@ashb
Copy link
Member

@ashb ashb commented Jun 1, 2020

scripts/ci/*.sh uses MY_DIR to mean scripts/ci, but in breeze MY_DIR
is the same as AIRFLOW_SOURCES. When jumping back-and-forth between
ci/utils.sh, breeze, and ci/ci*.sh it can be confusing to keep track
of what is what.

This changes breeze to use AIRFLOW_SOURCES to referrer to the top
level folder instead -- that means I don't have to keep as much context
in my head. I realise the MY_DIR was just "the folder of the script", but for some reason this was confusing my in breeze where AIRFLOW_SOURCES already existed.

In doing this I noticed a small "bug" -- we were creating ./scripts/ci/.kube folder for pre-commit runs.


Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

In case of fundamental code change, 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 UPDATING.md.
Read the Pull Request Guidelines for more information.

scripts/ci/*.sh uses MY_DIR to mean scripts/ci, but in `breeze` MY_DIR
is the same as AIRFLOW_SOURCES. When jumping back-and-forth between
ci/_utils.sh, breeze, and ci/ci_*.sh it can be confusing to keep track
of what is what.

This changes `breeze` to use `AIRFLOW_SOURCES` to referrer to the top
level folder instead -- that means I don't have to keep as much context
in my head
@ashb ashb requested a review from potiuk June 1, 2020 23:07
@potiuk potiuk merged commit 3dd81b7 into apache:master Jun 2, 2020
@ashb ashb deleted the breeze-var-name branch June 2, 2020 08:53
@potiuk potiuk added this to the Airflow 1.10.11 milestone Jun 5, 2020
potiuk pushed a commit that referenced this pull request Jun 9, 2020
…sh (#9098)

scripts/ci/*.sh uses MY_DIR to mean scripts/ci, but in `breeze` MY_DIR
is the same as AIRFLOW_SOURCES. When jumping back-and-forth between
ci/_utils.sh, breeze, and ci/ci_*.sh it can be confusing to keep track
of what is what.

This changes `breeze` to use `AIRFLOW_SOURCES` to referrer to the top
level folder instead -- that means I don't have to keep as much context
in my head

(cherry picked from commit 3dd81b7)
kaxil pushed a commit that referenced this pull request Jul 1, 2020
…sh (#9098)

scripts/ci/*.sh uses MY_DIR to mean scripts/ci, but in `breeze` MY_DIR
is the same as AIRFLOW_SOURCES. When jumping back-and-forth between
ci/_utils.sh, breeze, and ci/ci_*.sh it can be confusing to keep track
of what is what.

This changes `breeze` to use `AIRFLOW_SOURCES` to referrer to the top
level folder instead -- that means I don't have to keep as much context
in my head

(cherry picked from commit 3dd81b7)
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
…sh (apache#9098)

scripts/ci/*.sh uses MY_DIR to mean scripts/ci, but in `breeze` MY_DIR
is the same as AIRFLOW_SOURCES. When jumping back-and-forth between
ci/_utils.sh, breeze, and ci/ci_*.sh it can be confusing to keep track
of what is what.

This changes `breeze` to use `AIRFLOW_SOURCES` to referrer to the top
level folder instead -- that means I don't have to keep as much context
in my head

(cherry picked from commit 3dd81b7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants