I‘d also like to have auto-fixing included in CI. It is a classic pitfall and all that can be automated does not need to be a manual burden. Though I am not sure whether the plugin is able to use all the custom stuff as we also depend during execution on the CI image and docker. Besides security things this would be something that needs testing if it works.
TLDR: +1 opinion Sent from Outlook for iOS<https://aka.ms/o0ukef> ________________________________ From: Pankaj Koti <pankaj.k...@astronomer.io.INVALID> Sent: Saturday, December 30, 2023 7:50:10 AM To: dev@airflow.apache.org <dev@airflow.apache.org> Subject: Re: [DISCUSSION] Enabling `pre-commit.ci` application for Airflow 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://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpre-commit.ci%2F&data=05%7C02%7CJens.Scheffler%40de.bosch.com%7C7292ed56c2374a93eb1508dc09039ec0%7C0ae51e1907c84e4bbb6d648ee58410f4%7C0%7C0%7C638395158349294376%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=tgcyFz64S3IBLcAAYUYjswy7cqG%2FZ0KNbgRtSJyxOxQ%3D&reserved=0)<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://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FINFRA-25322&data=05%7C02%7CJens.Scheffler%40de.bosch.com%7C7292ed56c2374a93eb1508dc09039ec0%7C0ae51e1907c84e4bbb6d648ee58410f4%7C0%7C0%7C638395158349450599%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=mRyEImOqrqGXmbMxxLvPG5H%2F5J5CsnNdZH%2FYjJeRJLg%3D&reserved=0<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. > > >
