On 27-May-20 10:28 AM, Jerin Kollanukkaran wrote:
I think, original discussion[1] on this topic got lost in GitHub vs current
workflow.
I would like to propose GitHub "CODEOWNERS"[2] _LIKE_ scheme for DPDK workflow.
Current scheme:
- When we submit a patch to ml, someone(Tree maintainer[3]) needs to manually
delegate the patch to Tree maintainer in patchwork.
- Tree maintainer is not responsible for the review of the patch but only
responsible
for merging _after_ the review. That brings the obvious question on review
responsibility.
Proposed scheme:
- In order to improve review ownership, IMO, it is better the CI tools delegate
the patch to the actual maintainer(who is responsible for specific code in
MAINTAINERS file)
- I believe, it provides a sense of ownership, avoids last-minute surprise on
review responsibility and improve review traceability.
Implementation of the proposed scheme:
GitHub provides a bot for CODEOWNERS integration, Similar alternative is
possible with
patchwork with "auto delegation scheme" using the flowing methods:
a) https://patchwork.readthedocs.io/en/latest/usage/delegation/
b) https://patchwork.readthedocs.io/en/latest/usage/headers/
I think, option (a) would be relatively easy to change without introducing the
new tools.
Thoughts?
[1]
http://mails.dpdk.org/archives/dev/2020-May/168740.html
[2]
https://github.com/zephyrproject-rtos/zephyr/blob/master/CODEOWNERS
[3]
https://patchwork.dpdk.org/project/dpdk/
The "which patches should i review first" button is a huge +1000 from
me, as this has been a big issue i've had with current workflow for a
long time. Thomas has mentioned "Cc:" as a "fine grained" system to
assign patches, but the truth is, CC is not a good way of doing it
because i get CC'd in all kinds of stuff, and the important patches get
lost.
That said, i don't think it's that easy, because more often than not,
patches touch a lot of different areas, so a one line change in meson, a
test and a line in EAL gets half of DPDK maintainers CC'd into the
patch. I wonder if there is a mechanism for some kind of "threshold" for
assigning people to the patch - i.e. if a one-liner is half of the
changes in the patch, then maintainer gets CC'd, but if a one-liner is
just one of a thousand other unrelated lines, then perhaps there's no
need to CC the maintainer... or something along those lines :) there's a
machine learning project in here somewhere :D
--
Thanks,
Anatoly
