> Committers can merge PRs as they see fit. We don't have to wait for a specific code owner.
I must disagree with you. I know how open source projects works (I'm the maintainer of some) but in Airflow the problem is different. We have multiple providers that are responsible for their code but we're allowing anyone with contribution access to accept changes without notifying the code owners. It often happened that some provider introduced a major change in operators (and updated existing code) but then someone raised new PR with some changes, without this major change and it was merged without notifying given provider. It will be resolved if we move to separate repositories but for now we need to solve this issue in another way. See answer from Jarek to see if it's actually possible. In my exact case we introduced a new way of writing system tests, I've raised the PR migrating X test to new design and after my PR (which is still open) there was another PR that also modified the same file but using old design (which was merged - I see it's because the person raising and approving are from the same company, which helped to fasten the process). It's not the matter of simple rebase - the other PR shouldn't be approved since it's not up to our (current) community standards. And it wouldn't be approved if we were notified about this - we're responsible for our tests but our tests were modified without informing us. So I agree that "Commiters can merge PRs when they see fit" but I think in a repository that big as Airflow we need to at least make sure that people who know the given code the best (for example provider) are notified of proposed changes. pon., 11 kwi 2022 o 10:46 Elad Kalif <[email protected]> napisał(a): > Hi Bartłomiej, > > Due to the nature of the project multiple authors can work on the same > files and not even know one to another. > GitHub doesn't offer a feature to let the PR author know that there are > other open PRs requesting to change the same file. > > In general this is not "Priority to who opens first". It's "Priority for > what is ready". > > I feel your frustration. We all have been there. > We all had to rebase and resolve conflicts caused by other PRs. > > I don't think being a code owner is the solution here. > Committers can merge PRs as they see fit. We don't have to wait for a > specific code owner. > > On Mon, Apr 11, 2022 at 11:41 AM Jarek Potiuk <[email protected]> wrote: > >> This is a very good point :) and has surprising (at least to me) answer: >> >> We have two mechanisms now: >> >> * Labels by Boring Cyborg (not very helpful as you cannot selectively >> subscribe to a label): >> https://github.com/apache/airflow/blob/main/.github/boring-cyborg.yml >> >> * CODEOWNERS feature (this one is precisely what you want, because you >> are marked as reviewer and notified as CODEOWNER): >> https://github.com/apache/airflow/blob/main/.github/CODEOWNERS >> >> >> >> The problem with CODEOWNERS is (or rather was(!) ) that you could only >> add maintainers as CODEOWNERS. Up until recently the CODEOWNER >> documentation stated this: >> >> > The people you choose as code owners must have write permissions for >> the repository. When the code owner is a team, that team must have write >> permissions, even if all the individual members of the team already have >> write permissions directly, through organization membership, or through >> another team membership. >> >> However it seems that GitHub silently changed it :) and now it's a bit >> different: >> >> > Users must have *read* access to the repository and teams must have >> explicit write access, even if the team’s members already have access. You >> can also refer to a user by an email address that has been added to their >> account on GitHub.com, for example [email protected]. >> >> So - seems you can become a CODEOWNER even if you are not a >> maintainer!!!! YAY!. Something we really wanted to add for providers for a >> long time! >> I think you can achieve PRECISELY what you want - just make a PR to >> CODEOWNERS and add yourself to all Google Provider paths and we can test it. >> >> J. >> >> >> >> >> >> On Mon, Apr 11, 2022 at 10:07 AM Bartłomiej Hirsz <[email protected]> >> wrote: >> >>> Hi, >>> >>> Do we have any mechanism in Airflow to be notified on PRs touching >>> provider code hosted in the Airflow repository? I think that's not the case >>> - and it's related to recent discussions about providers in the core >>> repository (on dev list). I've had some problems in the past where I >>> discovered that there were PRs that changed behaviour of our provider and >>> it would be useful to be noticed on such PRs. >>> For example I've raised PR migrating Google GCS system tests to new >>> design: >>> https://github.com/apache/airflow/pull/22778 (5 days ago) >>> >>> Then someone raised PR extending the same files, but using old design: >>> https://github.com/apache/airflow/pull/22808 (4 days ago) >>> >>> The latter is already merged - now causing headache if it comes to >>> properly merging the changes. It could be avoided if there would be a code >>> owner set per files. Is it possible in Airflow? I know there is an >>> ownership file but previously only commiters (people with write access) >>> could be added. >>> >>> Best regards, >>> Bartłomiej Hirsz >>> >>
