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 exist. > * 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 know! 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: us...@infra.apache.org With regards, Apache Git Services
