potiuk commented on pull request #10651: URL: https://github.com/apache/airflow/pull/10651#issuecomment-683702514
> Should we also add a linter that checks those style rules? I think it's super difficult. We already had before `shellcheck` (which is also recommended by the Google Shell Guide BTW). And it catches (already did) a number of cases, but I am not sure there is a more comprehensive linter to check some of those. * it's rather difficult to say what is a "constant" and what is a "variable". In our case I marked as "constant" (i.e. CAPITALIZED) all the variables that result from choosing the appropriate `--switches` in Breeze. But they cannot be "defined" as constants, because they have to be set as defaults and only after all parsing of parameters is done we can mark them as "readonly" (and we do). * There are literally a few of those that are still "exported" but not constant (because they can change mid-way of pre-commits for example). `FORCE_ANSWERS_TO_QUESTIONS` is a good example. You can set it with switches (`--force-yes`) but even if you do not do it, in pre-commits you want to set it to `yes` after the build step successfully builds the image - so that you can run several pre-commits in a row and build the image only once - similarly `SKIP_BUILD_IMAGES` must be exported but it is not constant (but the fact that it is exported, still makes it capitalized) * the rules about `:-`, `-z`, `-n` are difficult to enforce. However those are enforced dynamically (not statically) with the `readonly` status. When you try to do `:=` with such variable you will get an error. So some of those rules are now verified dynamically. ---------------------------------------------------------------- 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]
