[GitHub] [airflow] potiuk commented on pull request #10695: Implement Google Shell Conventions for breeze script

[GitBox]

potiuk commented on pull request #10695:
URL: https://github.com/apache/airflow/pull/10695#issuecomment-685901279


   It was an interesting one. I fixed trap handling in this commit so that 
traps were "added" rather than "replaced". But they were added in a wrong 
order. Rather than putting them on a "stack" the add_trap method were adding 
them to the "list". The Traps shoudl follow the pattern of "stack" so thee last 
one should be executed first but it was reverse.
   
   One of the other changes I implemented few days earlier was making sure that 
the static checks output  only contain the pylint/mypy/etc and nothing else 
when pre-commit was run (the output before was cluttered with the informational 
message)  - that involved redirecting the output from pylint to the OUTPUT_LOG 
file.... And bad sequence of traps meant tha the file was deleted by the 
cleanup trap before it was printed out by the script_end trap
   
   That's why there was no output - the output file was deleted before it was 
printed
   
   This is the actual fix:
   
https://github.com/apache/airflow/pull/10695/commits/98756b64918c7dead2d2a0b6a49045db2740d18c
   
   
   I reproduced it with a bogus change:
   
   Before the fix:
   
   <img width="742" alt="Screenshot 2020-09-02 at 19 56 54" 
src="https://user-images.githubusercontent.com/595491/92019256-9a4e1400-ed56-11ea-9b77-58159ec781d3.png";>
   
   After the fix:
   
   <img width="758" alt="Screenshot 2020-09-02 at 19 57 09" 
src="https://user-images.githubusercontent.com/595491/92019272-9f12c800-ed56-11ea-9ff9-25684775e3d2.png";>
   
   


----------------------------------------------------------------
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:
us...@infra.apache.org