potiuk commented on a change in pull request #12735:
URL: https://github.com/apache/airflow/pull/12735#discussion_r533494733
##########
File path: breeze
##########
@@ -1361,9 +1361,9 @@ function breeze::parse_arguments() {
;;
*)
breeze::usage
- echo >&2
- echo >&2 "ERROR: Unknown command ${1}"
- echo >&2
+ echo
+ echo -e " \e[31mERROR: Unknown command\e[0m"
+ echo
Review comment:
> Why not on stderr anymore?
The main reason for printing it to stderr is that some tools will colour it
differently (red).It used to be standard way when terminals did not support
colors, but now when all of them do, it's rather useful for dev tools.
There are no other benefits unless the script/tool is used in Kubernetes,
etc. for running in production (I those cases intact) or when the output of
script is parsed by another tool (none of those scripts produced parseable
outout).
Those scripts are meant to be run in two cases only:
1) When user runs in it manually
2) When they run in CI environment
In both cases coloring part of the output manually is better than
redirecting to stderr nowadays. All the modern CI systems and all the terminals
for developers (including thsoe embedded in cloud tools and IDEs) support
coloured output. Similarly as refresh button in Airflow UI, let's move to the
XXI century.
##########
File path: breeze
##########
@@ -1361,9 +1361,9 @@ function breeze::parse_arguments() {
;;
*)
breeze::usage
- echo >&2
- echo >&2 "ERROR: Unknown command ${1}"
- echo >&2
+ echo
+ echo -e " \e[31mERROR: Unknown command\e[0m"
+ echo
Review comment:
> Why not on stderr anymore?
The main reason for printing it to stderr is that some tools will colour it
differently (red).It used to be standard way when terminals did not support
colors, but now when all of them do, it's rather useful for dev tools.
There are no other benefits unless the script/tool is used in Kubernetes,
etc. for running in production (I left those cases intact) or when the output
of script is parsed by another tool (none of those scripts produced parseable
outout).
Those scripts are meant to be run in two cases only:
1) When user runs in it manually
2) When they run in CI environment
In both cases coloring part of the output manually is better than
redirecting to stderr nowadays. All the modern CI systems and all the terminals
for developers (including thsoe embedded in cloud tools and IDEs) support
coloured output. Similarly as refresh button in Airflow UI, let's move to the
XXI century.
##########
File path: breeze
##########
@@ -1361,9 +1361,9 @@ function breeze::parse_arguments() {
;;
*)
breeze::usage
- echo >&2
- echo >&2 "ERROR: Unknown command ${1}"
- echo >&2
+ echo
+ echo -e " \e[31mERROR: Unknown command\e[0m"
+ echo
Review comment:
> Why not on stderr anymore?
The main reason for printing it to stderr is that some tools will colour it
differently (red).It used to be standard way when terminals did not support
colors, but now when all of them do, it's rather useful for dev tools.
There are no other benefits unless the script/tool is used in Kubernetes,
etc. for running in production (I left those cases intact) or when the output
of script is parsed by another tool (none of those scripts produced parseable
outout).
Those scripts are meant to be run in two cases only:
1) When user runs it manually
2) When they run in CI environment
In both cases coloring part of the output manually is better than
redirecting to stderr nowadays. All the modern CI systems and all the terminals
for developers (including thsoe embedded in cloud tools and IDEs) support
coloured output. Similarly as refresh button in Airflow UI, let's move to the
XXI century.
----------------------------------------------------------------
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]
