potiuk commented on a change in pull request #10708:
URL: https://github.com/apache/airflow/pull/10708#discussion_r482971647
##########
File path: breeze
##########
@@ -651,8 +651,8 @@ function breeze::prepare_command_files() {
# Prints detailed help for all commands and flags. Used to generate
documentation added to BREEZE.rst
# automatically.
#
-# Used globals:
-# ALL_BREEZE_COMMANDS
+# Used global variables:
+# _breeze_all_commands
Review comment:
Well. I thought about it - Technically it cannot be made constant (i.e.
we cannot make these read-only). We can not do it because:
* we need to source 'breeze-complete' before we enter Breeze so that the
'auto-complete' works - this happens in the .bashrc or .zshrc file
* we might want to source the same `breeeze-complete` potentially several
times - especially when you develop it or switch between different airflow
versions (1.10 -> master).
* most important - in Breeze I am re-sourcing ./breeze-complete always at
the beginning because I want to make sure that I am using the values from the
same directory as the `breeze` script. This way you can validate if for example
python versions are as expected in the version of Breeze script that you are
running currently (not the one that was sources in .bashrc, .zshrc). Those can
be different if you have several repositories with Breeze (as I have - with
1.10 and 2.0 for example, but I keep several worktrees of master as well)
So those values cannot be made `constant`. They might change. They also do
not need to be exported to other scripts - because we only use them in the
`breeze` script for validation and help messages - so no need to export them.
Since they are neither constant, nor exported - they are officially
'variables' and should be lower-case IMHO.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
