potiuk commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586169342
   > High level feedback since this was a topic earlier:
   > * the extra linting for the Dockerfile, which comes down to fairly trivial 
things like using cd in a command for one layer and using curl instead of wget, 
has led to several additional errors that are frustrating to deal with. If I 
knew the project perfectly sure I might have known these in advance, but for a 
new contributor that gets these bugs (which are somewhat subjective) that then 
has to figure them out and fix, it's an extra pain that simply doesn't need to 
   > * given the long duration of a PR, the addition of new linters / tests 
makes it even more confusing.
   > I'm happy to keep providing this feedback, and if you don't want it let me 
   Feedback is welcome :).
   I understand it's a bit of pain with linting, but I would treat it as a 
great learning experience. For me this is the best trainer I have - when I make 
a mistake or don't do something very well and can do better, I get feedback. 
That's the case with linting - I've learned a lot of good practices from 
linting - hadolint for Docker and especially shellcheck for bash. 
   And especially that those lint rules have very good explanation rationale 
and most of them has the effect that they prevent errors that others might have 
problems with. For example "using cd" has a good explanation and I think it's a 
fantastic practice to use WORKDIR instead of cd in Dockerfiles as it makes it 
more controllable - you always know which directory you are in without having 
to parse the shell statement. Here is more detailed explanation with example 
and rationale: https://github.com/hadolint/hadolint/wiki/DL3003 
   You might of course disagree with particular rules (and you have full right 
to do so) but we also have the rule in community "disagree but engage" - 
community decisions are more important than individual preferences. That's one 
of the things that I had to learn when I joined the community - there are a 
number of things I do not agree with but then after community decided otherwise 
i follow and even engage with). On the other hand there were a few rules that 
were annoying and we raised a proposal to remove them eventually, so there is 
still place for discussing them in the devlist - and if you have good reasoning 
why the rules should be removed, I am sure people who will be interested in it 
will have opinions and maybe decision will be made to disable those rules.
   * Regarding adding new checks - here again community is more important. When 
you look at it from your individual point of view, it's bad, but if you look at 
it from the community, there are more than 100 PRs opened at any time so we 
would never had a chance to improve - steady improvements are crucial, if you 
don't improve you actually deteriorate, so while annoying, it's also expected 
that after rebase you might have to adapt to some new rules. That's actually a 
sign of healthy project. Also if you had pre-commits installed and working, 
those "encoding" lines would be removed automatically for you. Whenever we add 
new rules we try to make them self-fixing and having pre-commit is one of the 
reasons for that.

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:

With regards,
Apache Git Services

Reply via email to