I very much like the concept. We have been using it actively for Astronomer code repositories for 1+ year already and it has helped us greatly (Thanks to Felix Uellendall for introducing this back then 🙂)
On Sat, 30 Dec 2023, 12:10 Jarek Potiuk, <ja...@potiuk.com> wrote: > FYI - Just now INFRA rejected the request on the basis of "code write" > access permissions the app needs. > > I'd still love to get feedback though on the concept - I am not giving up > that easily. We might still get it approved easily. We likely have some > ways we can get "auto-fixing" working for us. > > 1) I believe Github Applications now can use a bit different mechanism to > ask for permissions and it can have "required" and "optional" permissions > and I believe "Pull request write" should be enough (and I might attempt to > convince the maintainers of it to adapt it to our needs). > 2) Also, there is a "Pre-commit Lite Github Action" that we **might** be > able to use to achieve a similar effect (with some added complexity to our > `Pull Request Target` workflow. > > So I would still love to hear from others :) > > J. > > On Fri, Dec 29, 2023 at 11:52 PM Jarek Potiuk <ja...@potiuk.com> wrote: > > > Hello everyone, > > > > TL;DRl; I'd like to propose that we enable the pre-commit-ci GitHub > > application for Airflow repo. According to how I understand it works, it > > should greatly reduce friction (especially for new contributors) for > > passing the quality gates for our pre-commits. That is - if we get the > > approval for pre-commit-ci application from the ASF infra team. > > > > Some more context: > > > > We use and love (well some of us do, some of us likely hate) the > > pre-commit as a quality gate for our checks. We have been using it for > > years for local checks and CI integration and we have ~60 custom > precommits > > and in total we use about 100 pre-commit checks as our "quality" gates > > > > However, using `standard` pre-commit (that is a de-facto standard in > > Python world) has a nice property of 'standing on the shoulders of > giants'. > > There is one thing that few of us are aware of, that there is a way to > > reduce friction for pre-commits that are not only flagging errors but can > > also fix them. If we get the `pre-commit-ci` application ( > > https://pre-commit.ci/) approved for our repo from the ASF infra team it > > - in theory - should be able to AUTO-FIX PRs that are not passing the > > pre-commits (and can be automatically corrected). > > > > Yep. You read it right. No more asking a new contributor "please fix > > static checks" - PRs that have auto-fixable pre-commit failures will be > > fixed automatically. > > > > For example when you make a PR that does not pass "ruff" formatting, the > > application should automatically amend the PR and FIX IT. We have quite a > > number of such PRs from first-time contributors, but also a number of > > seasoned contributors (including myself) occasionally send a PR that does > > not pass an auto-fixable static check. This can happen with a few > scenarios > > (rebasing, or correcting a PR by applying a suggestion from review and a > > few other scenarios). > > > > It can be a little strange to see your PR corrected by a bot though, so I > > am reaching out here to see if you think it is a good idea. I also > opened a > > JIRA request to approve the application (but I made a comment that it > > should be pending the discussion here): > > https://issues.apache.org/jira/browse/INFRA-25322 - it will likely > > require to slightly change our workflows to make it works as well. > > > > Do you think it's a good idea to have it enabled? Maybe it will be too > > much for our contributors and they will be surprised to see it happening? > > > > WDYT? > > > > J. > > >
