On 5/2/2024 6:57 AM, Leif Lindholm wrote:
On 2024-05-02 04:08, Michael Kubacki wrote:
Thank you for this proposal. We've been anticipating this change for
years and are excited to help support it.
Here's some items we'd like to raise for feedback that we could help
implement. Many could likely be done in time for the transition.
1. Automate reviewers - We've discussed CODEOWNERS in the past.
However, a simpler approach (in maintaining/syncing less files) would
be to use Maintainers.txt directly with a GitHub workflow since the
file already contains GitHub IDs.
That would be ideal. I know Mike worked on autogenerating CODEOWNERS
from Maintainers.txt, but ultimately the latter supports more flexible
use of wildcards (things like */AArch64/ currently requires reconciling
against the repo contents).
2. Make PR completion contingent on a GitHub review from at least one
package maintainer/reviewer for each package in the PR.
Yes.
3. Dependabot is already used today to automatically create PRs when
dependencies like pip modules have updates. To allow this to more
effectively keep dependencies up-to-date, allow dependabot PRs to be
completed (after normal acceptance criteria like CI and review
requirements) without a separate human creating a duplicate PR.
I am not sure what this means in practice :)
This doesn't sound like one we need to worry about before switchover
though.
It was a minor point to reduce extra work for keeping dependencies
up-to-date. It will naturally lead to a PR review in the new process
where we can discuss it further in the review.
4. Potentially warn users (with an automated comment on the PR) if
they add a push label to a PR that is less than 24 hours old.
That sounds good.
Is there any way to prevent force-pushes within 24h of previous push?
That would make setting up a transitional review-scraper less lossy.
We can. There have been cases in the past where we need to critically
get a change in to unblock CI for example so I didn't want to complicate
that initially. This could serve as a reminder to those with push access
in the meantime if we'd like.
5. Leave reminder comments on PRs with absolutely no activity after
some agreed upon time so reviewers are notified to review the PR
without the submitter having to watch it and send notifications.
Yes. But should take priority below 1, 2, and 4. Unless you have a
pre-cooked thing to drop in of course.
Your priorities look good.
6. Leave reminder comments on PRs that meet all requirements to be
completed (all reviews accounted for and status checks pass) but are
still open so those on the PR are notified to complete it without the
submitter having to manually watch and send reminders.
Not a response to this, but triggered by reading this:
Is there any way to approve changes within a PR on a commit by commit
basis?
Unfortunately not. We typically resolve all conversations before giving
an approval that would apply to all relevant commits for the given reviewer.
7. We are happy to help with process documentation.
Always appreciated,thanks.
Regards,
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118557): https://edk2.groups.io/g/devel/message/118557
Mute This Topic: https://groups.io/mt/105847510/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
