DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jerin Jacob <jerinjacobk@gmail.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Gaëtan Rivet <grive@u256.net>,
	 Jerin Kollanukkaran <jerinj@marvell.com>,
	dpdk-dev <dev@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>,
	 "david.marchand@redhat.com" <david.marchand@redhat.com>,
	Maxime Coquelin <maxime.coquelin@redhat.com>,
	 "cristian.dumitrescu@intel.com" <cristian.dumitrescu@intel.com>,
	"akhil.goyal@nxp.com" <akhil.goyal@nxp.com>,
	 "rasland@mellanox.com" <rasland@mellanox.com>,
	"xiaolong.ye@intel.com" <xiaolong.ye@intel.com>,
	 "ajit.khaparde@broadcom.com" <ajit.khaparde@broadcom.com>,
	 "arybchenko@solarflare.com" <arybchenko@solarflare.com>,
	"Burakov, Anatoly" <anatoly.burakov@intel.com>,
	 "techboard@dpdk.org" <techboard@dpdk.org>
Subject: Re: [dpdk-dev] Suggestion to improve the code review
Date: Tue, 2 Jun 2020 21:53:29 +0530
Message-ID: <CALBAE1ODM9KmzfUX+cpvVR_NEkv41yap5WZkZVa6HvG4jdKhbA@mail.gmail.com> (raw)
In-Reply-To: <bff6205a-64c2-db7d-5ca1-9423167f4830@intel.com>

On Tue, Jun 2, 2020 at 8:27 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 6/2/2020 1:27 PM, Jerin Jacob wrote:
> > On Wed, May 27, 2020 at 3:38 PM Gaëtan Rivet <grive@u256.net> wrote:
> >>
> >> On 27/05/20 09:28 +0000, 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/
> >>>
> >>
> >> Hi,
> >>
> >> +1 from me. People would be able to list current assigned tasks through
> >> pwclient. It would help reviews IMO.
> >
> > So far no objection to this proposal. Any other thoughts from anyone?
> > especially from the code maintainers.
> >
> > Thomas, Any input as patchwork maintainer. This would boil down to the
> > following change in patchwork.
> >
> > 1) Add code maintainers are maintainers in patchwork.
> > 2) Enable existing auto delegation[1] feature of Patchwork
> > [1]
> > a) https://patchwork.readthedocs.io/en/latest/usage/delegation/
> > b) https://patchwork.readthedocs.io/en/latest/usage/headers/
> >
> > The suggested process is:
> > # When a patch gets submitted to ml, patchwork finds the code owner
> > based on the MAINTAINER file using the auto delegation feature.
> > # The code maintainer will be responsible for the "review" of that
> > patch and patch will be delegate will code owner using auto delegation
> > feature.
> > # If multiple code maintainers operate on the same patch, "each code
> > maintainer" can assign to "other code maintainer" once he is done with
> > the review.
> > # The existing review process will be followed as is, just that we are
> > adding code maintainer have primary review responsibility for the
> > patch and expressing in the patchwork.
> > # Based on the Ack's received and/or when code owner is happy with
> > changes, he/she can change the state  to "Awaiting upstream" and
> > assign to respective
> > tree maintainer.
> > # Finally, Tree maintainer will merge the patch to respective tree and
> > make the state as  "Accepted"
> >
>
> +1 from me, this can help maintainers to figure out patches waiting for their
> review.
>
> Did you have a chance to test auto delegation, will it work for us?

I think, it can be done in two ways

a) https://patchwork.readthedocs.io/en/latest/usage/delegation/
b) https://patchwork.readthedocs.io/en/latest/usage/headers/

Option (a) need patchwork admin access and no dependency on email
client nor separate step[1]. I think, only Thomas only has access to
that.
I tested the option (b). It is not working, it is not straight forward
as we need to specific header to email[1]
Based on my debugging, Even though when I did "add-header", it is not
showing up on received email. Somewhere it is getting removed[2]

[1]
git send-email --to dev@dpdk.org --add-header="X-Patchwork-Delegate:
ferruh.yigit@intel.com"
0001-test-test-patch-for-checking-patchwork-auto-delegati.patch
[2]
http://patches.dpdk.org/patch/70749/

  reply	other threads:[~2020-06-02 16:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-27  9:28 Jerin Kollanukkaran
2020-05-27  9:59 ` Burakov, Anatoly
2020-05-27 11:27   ` Jerin Jacob
2020-05-27 10:08 ` Gaëtan Rivet
2020-06-02 12:27   ` Jerin Jacob
2020-06-02 14:57     ` Ferruh Yigit
2020-06-02 16:23       ` Jerin Jacob [this message]
2020-06-03 13:09         ` Ferruh Yigit
2020-06-03 13:56           ` Jerin Jacob

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALBAE1ODM9KmzfUX+cpvVR_NEkv41yap5WZkZVa6HvG4jdKhbA@mail.gmail.com \
    --to=jerinjacobk@gmail.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=akhil.goyal@nxp.com \
    --cc=anatoly.burakov@intel.com \
    --cc=arybchenko@solarflare.com \
    --cc=cristian.dumitrescu@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=grive@u256.net \
    --cc=jerinj@marvell.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=rasland@mellanox.com \
    --cc=techboard@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=xiaolong.ye@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git