DPDK CI discussions
 help / color / mirror / Atom feed
* Adding Series Dependency to Patchwork
@ 2024-02-22 19:51 Adam Hassick
  2024-04-04 13:50 ` Patrick Robb
  0 siblings, 1 reply; 17+ messages in thread
From: Adam Hassick @ 2024-02-22 19:51 UTC (permalink / raw)
  To: ci

Hi All,

There has been a discussion on GitHub between myself and others about
adding support for series dependency to Patchwork. This would involve
adding the feature to the Patchwork dashboard and git-pw. However, it
seems like the maintainer would like us to do much of the required
implementation work. I estimate, with a low level of certainty, that
it would consume 35 to 50 hours of developer time.

The active issue relating to this feature can be found here, as well
as the archived one:
https://github.com/getpatchwork/patchwork/issues/583

Would it be worth it for us here at the Community Lab to contribute
upstream to Patchwork? Implementing the feature in the Patchwork
project will make our method for defining series dependencies the
standard, and it may lift some of the burden of maintaining this
feature off of us.

Regards,
Adam Hassick

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-02-22 19:51 Adding Series Dependency to Patchwork Adam Hassick
@ 2024-04-04 13:50 ` Patrick Robb
  2024-04-05  6:41   ` Thomas Monjalon
  0 siblings, 1 reply; 17+ messages in thread
From: Patrick Robb @ 2024-04-04 13:50 UTC (permalink / raw)
  To: ci, NBU-Contact-Thomas Monjalon (EXTERNAL)
  Cc: Adam Hassick, David Marchand, Aaron Conole

Hi Thomas,

Adam explains more completely in the preceding email, but he has
talked with the patchwork project maintainer, and determined the
needed next steps to do work adding depends-on support to patchwork
server. And he provides an estimate (35-50 hours of his time) for
adding this feature to pw.

What we want is a community opinion on using Community Lab resources
(our developer time) towards this effort on patchwork project. I'm
going to you, because you were the person who originally raised the
depends on issue and adding this support to pw server.

Given his time estimate, do you endorse this work, or do you prefer we
focus on our SOW items and other community requests which are in queue
right now? Thanks.

On Thu, Feb 22, 2024 at 2:51 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
>
> Hi All,
>
> There has been a discussion on GitHub between myself and others about
> adding support for series dependency to Patchwork. This would involve
> adding the feature to the Patchwork dashboard and git-pw. However, it
> seems like the maintainer would like us to do much of the required
> implementation work. I estimate, with a low level of certainty, that
> it would consume 35 to 50 hours of developer time.
>
> The active issue relating to this feature can be found here, as well
> as the archived one:
> https://github.com/getpatchwork/patchwork/issues/583
>
> Would it be worth it for us here at the Community Lab to contribute
> upstream to Patchwork? Implementing the feature in the Patchwork
> project will make our method for defining series dependencies the
> standard, and it may lift some of the burden of maintaining this
> feature off of us.
>
> Regards,
> Adam Hassick

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-04-04 13:50 ` Patrick Robb
@ 2024-04-05  6:41   ` Thomas Monjalon
  2024-04-11 14:18     ` Aaron Conole
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2024-04-05  6:41 UTC (permalink / raw)
  To: Patrick Robb, Adam Hassick; +Cc: ci, David Marchand, Aaron Conole

Hello,

Yes I still think it is a valuable work to achieve.
If Aaron agrees, I wish you can complete this feature upstream.

Thank you


04/04/2024 15:50, Patrick Robb:
> Hi Thomas,
> 
> Adam explains more completely in the preceding email, but he has
> talked with the patchwork project maintainer, and determined the
> needed next steps to do work adding depends-on support to patchwork
> server. And he provides an estimate (35-50 hours of his time) for
> adding this feature to pw.
> 
> What we want is a community opinion on using Community Lab resources
> (our developer time) towards this effort on patchwork project. I'm
> going to you, because you were the person who originally raised the
> depends on issue and adding this support to pw server.
> 
> Given his time estimate, do you endorse this work, or do you prefer we
> focus on our SOW items and other community requests which are in queue
> right now? Thanks.
> 
> On Thu, Feb 22, 2024 at 2:51 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
> >
> > Hi All,
> >
> > There has been a discussion on GitHub between myself and others about
> > adding support for series dependency to Patchwork. This would involve
> > adding the feature to the Patchwork dashboard and git-pw. However, it
> > seems like the maintainer would like us to do much of the required
> > implementation work. I estimate, with a low level of certainty, that
> > it would consume 35 to 50 hours of developer time.
> >
> > The active issue relating to this feature can be found here, as well
> > as the archived one:
> > https://github.com/getpatchwork/patchwork/issues/583
> >
> > Would it be worth it for us here at the Community Lab to contribute
> > upstream to Patchwork? Implementing the feature in the Patchwork
> > project will make our method for defining series dependencies the
> > standard, and it may lift some of the burden of maintaining this
> > feature off of us.
> >
> > Regards,
> > Adam Hassick
> 






^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-04-05  6:41   ` Thomas Monjalon
@ 2024-04-11 14:18     ` Aaron Conole
  2024-04-11 14:33       ` Patrick Robb
  0 siblings, 1 reply; 17+ messages in thread
From: Aaron Conole @ 2024-04-11 14:18 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Patrick Robb, Adam Hassick, ci, David Marchand

Thomas Monjalon <thomas@monjalon.net> writes:

> Hello,
>
> Yes I still think it is a valuable work to achieve.
> If Aaron agrees, I wish you can complete this feature upstream.

okay by me

> Thank you
>
>
> 04/04/2024 15:50, Patrick Robb:
>> Hi Thomas,
>> 
>> Adam explains more completely in the preceding email, but he has
>> talked with the patchwork project maintainer, and determined the
>> needed next steps to do work adding depends-on support to patchwork
>> server. And he provides an estimate (35-50 hours of his time) for
>> adding this feature to pw.
>> 
>> What we want is a community opinion on using Community Lab resources
>> (our developer time) towards this effort on patchwork project. I'm
>> going to you, because you were the person who originally raised the
>> depends on issue and adding this support to pw server.
>> 
>> Given his time estimate, do you endorse this work, or do you prefer we
>> focus on our SOW items and other community requests which are in queue
>> right now? Thanks.
>> 
>> On Thu, Feb 22, 2024 at 2:51 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
>> >
>> > Hi All,
>> >
>> > There has been a discussion on GitHub between myself and others about
>> > adding support for series dependency to Patchwork. This would involve
>> > adding the feature to the Patchwork dashboard and git-pw. However, it
>> > seems like the maintainer would like us to do much of the required
>> > implementation work. I estimate, with a low level of certainty, that
>> > it would consume 35 to 50 hours of developer time.
>> >
>> > The active issue relating to this feature can be found here, as well
>> > as the archived one:
>> > https://github.com/getpatchwork/patchwork/issues/583
>> >
>> > Would it be worth it for us here at the Community Lab to contribute
>> > upstream to Patchwork? Implementing the feature in the Patchwork
>> > project will make our method for defining series dependencies the
>> > standard, and it may lift some of the burden of maintaining this
>> > feature off of us.
>> >
>> > Regards,
>> > Adam Hassick
>> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-04-11 14:18     ` Aaron Conole
@ 2024-04-11 14:33       ` Patrick Robb
  2024-07-12 20:15         ` Adam Hassick
  0 siblings, 1 reply; 17+ messages in thread
From: Patrick Robb @ 2024-04-11 14:33 UTC (permalink / raw)
  To: Aaron Conole; +Cc: Thomas Monjalon, Adam Hassick, ci, David Marchand

Perfect. Then we will proceed.

On Thu, Apr 11, 2024 at 10:18 AM Aaron Conole <aconole@redhat.com> wrote:
>
> Thomas Monjalon <thomas@monjalon.net> writes:
>
> > Hello,
> >
> > Yes I still think it is a valuable work to achieve.
> > If Aaron agrees, I wish you can complete this feature upstream.
>
> okay by me
>
> > Thank you
> >
> >
> > 04/04/2024 15:50, Patrick Robb:
> >> Hi Thomas,
> >>
> >> Adam explains more completely in the preceding email, but he has
> >> talked with the patchwork project maintainer, and determined the
> >> needed next steps to do work adding depends-on support to patchwork
> >> server. And he provides an estimate (35-50 hours of his time) for
> >> adding this feature to pw.
> >>
> >> What we want is a community opinion on using Community Lab resources
> >> (our developer time) towards this effort on patchwork project. I'm
> >> going to you, because you were the person who originally raised the
> >> depends on issue and adding this support to pw server.
> >>
> >> Given his time estimate, do you endorse this work, or do you prefer we
> >> focus on our SOW items and other community requests which are in queue
> >> right now? Thanks.
> >>
> >> On Thu, Feb 22, 2024 at 2:51 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
> >> >
> >> > Hi All,
> >> >
> >> > There has been a discussion on GitHub between myself and others about
> >> > adding support for series dependency to Patchwork. This would involve
> >> > adding the feature to the Patchwork dashboard and git-pw. However, it
> >> > seems like the maintainer would like us to do much of the required
> >> > implementation work. I estimate, with a low level of certainty, that
> >> > it would consume 35 to 50 hours of developer time.
> >> >
> >> > The active issue relating to this feature can be found here, as well
> >> > as the archived one:
> >> > https://github.com/getpatchwork/patchwork/issues/583
> >> >
> >> > Would it be worth it for us here at the Community Lab to contribute
> >> > upstream to Patchwork? Implementing the feature in the Patchwork
> >> > project will make our method for defining series dependencies the
> >> > standard, and it may lift some of the burden of maintaining this
> >> > feature off of us.
> >> >
> >> > Regards,
> >> > Adam Hassick
> >>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-04-11 14:33       ` Patrick Robb
@ 2024-07-12 20:15         ` Adam Hassick
  2024-07-19 15:32           ` Patrick Robb
  2024-07-19 17:40           ` Ferruh Yigit
  0 siblings, 2 replies; 17+ messages in thread
From: Adam Hassick @ 2024-07-12 20:15 UTC (permalink / raw)
  To: Patrick Robb; +Cc: Aaron Conole, Thomas Monjalon, ci, David Marchand

Hi All,

We've gotten a review of our initial submission to add the dependency
parsing to the Patchwork dashboard. Stephen recommends that we change
our format to use the message ID of patches or cover letters rather
than the ID of the patch or series in the database. So, instead of
adding a dependency by adding "Depends-on: series-5678" one would add
"Depends-on: <20240712120000-1-user@example.com>". We can keep the
option of using the patchwork web URLs that was discussed in the
original issue on GitHub.

The main reasoning for this is that our format doesn't make it clear
exactly what the dependency is or where it's found outside of the
context of Patchwork. This discussion can be viewed here:
http://patchwork.ozlabs.org/project/patchwork/patch/20240617221900.156155-3-ahassick@iol.unh.edu/

Does this change sound reasonable?

Regards,
Adam

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-12 20:15         ` Adam Hassick
@ 2024-07-19 15:32           ` Patrick Robb
  2024-07-19 17:43             ` Ferruh Yigit
  2024-07-19 17:40           ` Ferruh Yigit
  1 sibling, 1 reply; 17+ messages in thread
From: Patrick Robb @ 2024-07-19 15:32 UTC (permalink / raw)
  To: Adam Hassick; +Cc: Aaron Conole, Thomas Monjalon, ci, David Marchand

My view is that if the patchwork project is trying to get away from
integer based series dependencies and use a more authoritative key
like the message ID when associating series, we should follow suit in
terms of DPDK submission guidelines. So I think the "Depends-on:
<20240712120000-1-user@example.com>" format sounds fine.

But, obviously this will require a policy change and update to the
DPDK submission guidelines, so I'm curious what others outside of UNH
think.

I will add it to the discussion list for next Thursday's CI meeting.

Thanks Adam.

On Fri, Jul 12, 2024 at 4:14 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
>
> Hi All,
>
> We've gotten a review of our initial submission to add the dependency
> parsing to the Patchwork dashboard. Stephen recommends that we change
> our format to use the message ID of patches or cover letters rather
> than the ID of the patch or series in the database. So, instead of
> adding a dependency by adding "Depends-on: series-5678" one would add
> "Depends-on: <20240712120000-1-user@example.com>". We can keep the
> option of using the patchwork web URLs that was discussed in the
> original issue on GitHub.
>
> The main reasoning for this is that our format doesn't make it clear
> exactly what the dependency is or where it's found outside of the
> context of Patchwork. This discussion can be viewed here:
> http://patchwork.ozlabs.org/project/patchwork/patch/20240617221900.156155-3-ahassick@iol.unh.edu/
>
> Does this change sound reasonable?
>
> Regards,
> Adam

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-12 20:15         ` Adam Hassick
  2024-07-19 15:32           ` Patrick Robb
@ 2024-07-19 17:40           ` Ferruh Yigit
  2024-07-22 16:16             ` Adam Hassick
  1 sibling, 1 reply; 17+ messages in thread
From: Ferruh Yigit @ 2024-07-19 17:40 UTC (permalink / raw)
  To: Adam Hassick, Patrick Robb
  Cc: Aaron Conole, Thomas Monjalon, ci, David Marchand

On 7/12/2024 9:15 PM, Adam Hassick wrote:
> Hi All,
> 
> We've gotten a review of our initial submission to add the dependency
> parsing to the Patchwork dashboard. Stephen recommends that we change
> our format to use the message ID of patches or cover letters rather
> than the ID of the patch or series in the database. So, instead of
> adding a dependency by adding "Depends-on: series-5678" one would add
> "Depends-on: <20240712120000-1-user@example.com>". We can keep the
> option of using the patchwork web URLs that was discussed in the
> original issue on GitHub.
> 

I wasn't aware that patchwork has target to abstract/hide the integer
IDs, in this case since we want to enable this feature in the patchwork,
it makes sense to follow their path.
No need to push for integer IDs, that was something we thought as an
option, it is not crucial for us, we can change it.

But there is another thing Stephen mentioned, perhaps using URL instead
of message ID.
Even message ID gives more data, it may still require some effort for
someone investigating the patch to access to that dependent
patch/series. But using URL gives instant access.
If using URL doesn't make like harder for the patchwork implementation,
I am for using URL. From patch developer perspective, I think providing
message ID or patchwork URL are similar level of hassle.

If we go with the URL option, does is still required to differentiate as
"patch-xxx" or "series-yyy", previously they were different IDs, but
with URL can patchwork deduce if it is series or patch? If so this can
bring a simplification.


> The main reasoning for this is that our format doesn't make it clear
> exactly what the dependency is or where it's found outside of the
> context of Patchwork. This discussion can be viewed here:
> http://patchwork.ozlabs.org/project/patchwork/patch/20240617221900.156155-3-ahassick@iol.unh.edu/
> 
> Does this change sound reasonable?
> 
> Regards,
> Adam


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-19 15:32           ` Patrick Robb
@ 2024-07-19 17:43             ` Ferruh Yigit
  0 siblings, 0 replies; 17+ messages in thread
From: Ferruh Yigit @ 2024-07-19 17:43 UTC (permalink / raw)
  To: Patrick Robb, Adam Hassick
  Cc: Aaron Conole, Thomas Monjalon, ci, David Marchand

On 7/19/2024 4:32 PM, Patrick Robb wrote:
> My view is that if the patchwork project is trying to get away from
> integer based series dependencies and use a more authoritative key
> like the message ID when associating series, we should follow suit in
> terms of DPDK submission guidelines. So I think the "Depends-on:
> <20240712120000-1-user@example.com>" format sounds fine.
> 
> But, obviously this will require a policy change and update to the
> DPDK submission guidelines, so I'm curious what others outside of UNH
> think.
> 

I am not much worried of the changing PDK submission guidelines, this is
not used very actively anyway, so impact will be low.


> I will add it to the discussion list for next Thursday's CI meeting.
> 
> Thanks Adam.
> 
> On Fri, Jul 12, 2024 at 4:14 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
>>
>> Hi All,
>>
>> We've gotten a review of our initial submission to add the dependency
>> parsing to the Patchwork dashboard. Stephen recommends that we change
>> our format to use the message ID of patches or cover letters rather
>> than the ID of the patch or series in the database. So, instead of
>> adding a dependency by adding "Depends-on: series-5678" one would add
>> "Depends-on: <20240712120000-1-user@example.com>". We can keep the
>> option of using the patchwork web URLs that was discussed in the
>> original issue on GitHub.
>>
>> The main reasoning for this is that our format doesn't make it clear
>> exactly what the dependency is or where it's found outside of the
>> context of Patchwork. This discussion can be viewed here:
>> http://patchwork.ozlabs.org/project/patchwork/patch/20240617221900.156155-3-ahassick@iol.unh.edu/
>>
>> Does this change sound reasonable?
>>
>> Regards,
>> Adam


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-19 17:40           ` Ferruh Yigit
@ 2024-07-22 16:16             ` Adam Hassick
  2024-07-22 16:28               ` Patrick Robb
  0 siblings, 1 reply; 17+ messages in thread
From: Adam Hassick @ 2024-07-22 16:16 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Patrick Robb, Aaron Conole, Thomas Monjalon, ci, David Marchand

On Fri, Jul 19, 2024 at 1:41 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 7/12/2024 9:15 PM, Adam Hassick wrote:
> > Hi All,
> >
> > We've gotten a review of our initial submission to add the dependency
> > parsing to the Patchwork dashboard. Stephen recommends that we change
> > our format to use the message ID of patches or cover letters rather
> > than the ID of the patch or series in the database. So, instead of
> > adding a dependency by adding "Depends-on: series-5678" one would add
> > "Depends-on: <20240712120000-1-user@example.com>". We can keep the
> > option of using the patchwork web URLs that was discussed in the
> > original issue on GitHub.
> >
>
> I wasn't aware that patchwork has target to abstract/hide the integer
> IDs, in this case since we want to enable this feature in the patchwork,
> it makes sense to follow their path.
> No need to push for integer IDs, that was something we thought as an
> option, it is not crucial for us, we can change it.

Ok, that's good to hear. I can start preparing the v2 patch to switch
over to using the message IDs.

> But there is another thing Stephen mentioned, perhaps using URL instead
> of message ID.
> Even message ID gives more data, it may still require some effort for
> someone investigating the patch to access to that dependent
> patch/series. But using URL gives instant access.
> If using URL doesn't make like harder for the patchwork implementation,
> I am for using URL. From patch developer perspective, I think providing
> message ID or patchwork URL are similar level of hassle.

My current implementation supports the URL format and the current DPDK
format as values for the Depends-on tag. If I switch out the current
DPDK format with the message IDs, then we can support both.

> If we go with the URL option, does is still required to differentiate as
> "patch-xxx" or "series-yyy", previously they were different IDs, but
> with URL can patchwork deduce if it is series or patch? If so this can
> bring a simplification.

No, you can just paste the URL and the Django URL resolver will figure
out whether it points at a patch or a series. No need to differentiate
with the URLs.

That's also true of the message ID option too. There isn't much of a
point in differentiating patch/series message IDs because series do
not reliably have an email associated with them.


> > The main reasoning for this is that our format doesn't make it clear
> > exactly what the dependency is or where it's found outside of the
> > context of Patchwork. This discussion can be viewed here:
> > http://patchwork.ozlabs.org/project/patchwork/patch/20240617221900.156155-3-ahassick@iol.unh.edu/
> >
> > Does this change sound reasonable?
> >
> > Regards,
> > Adam
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-22 16:16             ` Adam Hassick
@ 2024-07-22 16:28               ` Patrick Robb
  2024-07-23 12:41                 ` Aaron Conole
       [not found]                 ` <6b425d90-78b2-497b-958c-9d36e2ba6e3b@amd.com>
  0 siblings, 2 replies; 17+ messages in thread
From: Patrick Robb @ 2024-07-22 16:28 UTC (permalink / raw)
  To: Adam Hassick
  Cc: Ferruh Yigit, Aaron Conole, Thomas Monjalon, ci, David Marchand

On Mon, Jul 22, 2024 at 12:15 PM Adam Hassick <ahassick@iol.unh.edu> wrote:

> > If we go with the URL option, does is still required to differentiate as
> > "patch-xxx" or "series-yyy", previously they were different IDs, but
> > with URL can patchwork deduce if it is series or patch? If so this can
> > bring a simplification.
>
> No, you can just paste the URL and the Django URL resolver will figure
> out whether it points at a patch or a series. No need to differentiate
> with the URLs.
>
> That's also true of the message ID option too. There isn't much of a
> point in differentiating patch/series message IDs because series do
> not reliably have an email associated with them.

Sounds good. I want to highlight again for the ci group that all
dependencies will be series dependencies, regardless of whether
"patch-xxx" or "series-yyy" is used. If a patch message id or url is
submitted, it will be mapped to its series url for the dependency.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-22 16:28               ` Patrick Robb
@ 2024-07-23 12:41                 ` Aaron Conole
       [not found]                 ` <6b425d90-78b2-497b-958c-9d36e2ba6e3b@amd.com>
  1 sibling, 0 replies; 17+ messages in thread
From: Aaron Conole @ 2024-07-23 12:41 UTC (permalink / raw)
  To: Patrick Robb
  Cc: Adam Hassick, Ferruh Yigit, Thomas Monjalon, ci, David Marchand

Patrick Robb <probb@iol.unh.edu> writes:

> On Mon, Jul 22, 2024 at 12:15 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
>
>> > If we go with the URL option, does is still required to differentiate as
>> > "patch-xxx" or "series-yyy", previously they were different IDs, but
>> > with URL can patchwork deduce if it is series or patch? If so this can
>> > bring a simplification.
>>
>> No, you can just paste the URL and the Django URL resolver will figure
>> out whether it points at a patch or a series. No need to differentiate
>> with the URLs.
>>
>> That's also true of the message ID option too. There isn't much of a
>> point in differentiating patch/series message IDs because series do
>> not reliably have an email associated with them.
>
> Sounds good. I want to highlight again for the ci group that all
> dependencies will be series dependencies, regardless of whether
> "patch-xxx" or "series-yyy" is used. If a patch message id or url is
> submitted, it will be mapped to its series url for the dependency.

Thanks for the heads up - it means we will probably need to change
things in the robot to make sure we support that format when it gets
accepted.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
       [not found]                 ` <6b425d90-78b2-497b-958c-9d36e2ba6e3b@amd.com>
@ 2024-07-23 15:36                   ` Patrick Robb
  2024-07-23 16:08                     ` Ferruh Yigit
  0 siblings, 1 reply; 17+ messages in thread
From: Patrick Robb @ 2024-07-23 15:36 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Adam Hassick, ci, Aaron Conole

On Tue, Jul 23, 2024 at 11:31 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 7/22/2024 5:28 PM, Patrick Robb wrote:
> > On Mon, Jul 22, 2024 at 12:15 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
> >
> >>> If we go with the URL option, does is still required to differentiate as
> >>> "patch-xxx" or "series-yyy", previously they were different IDs, but
> >>> with URL can patchwork deduce if it is series or patch? If so this can
> >>> bring a simplification.
> >>
> >> No, you can just paste the URL and the Django URL resolver will figure
> >> out whether it points at a patch or a series. No need to differentiate
> >> with the URLs.
> >>
> >> That's also true of the message ID option too. There isn't much of a
> >> point in differentiating patch/series message IDs because series do
> >> not reliably have an email associated with them.
> >
> > Sounds good. I want to highlight again for the ci group that all
> > dependencies will be series dependencies, regardless of whether
> > "patch-xxx" or "series-yyy" is used. If a patch message id or url is
> > submitted, it will be mapped to its series url for the dependency.
> >
>
> Are you planning to keep the 'patch' or 'series' part, why not change
> the syntax as:
>
> Depends-on: <message-ID>
> or
> Depends-on: <patchwork URL>
>

Good point. Yes, there is no reason to keep the "patch" or "series"
prefix to the value.

>
> And is there a benefit to support both "message-ID" and "patchwork URL",
> so why not just:
> Depends-on: <patchwork URL>
>

Maybe Adam can answer, but I think his intention was to support both
formats, to provide more flexibility for users.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-23 15:36                   ` Patrick Robb
@ 2024-07-23 16:08                     ` Ferruh Yigit
  2024-07-24 15:07                       ` Adam Hassick
  0 siblings, 1 reply; 17+ messages in thread
From: Ferruh Yigit @ 2024-07-23 16:08 UTC (permalink / raw)
  To: Patrick Robb; +Cc: Adam Hassick, ci, Aaron Conole

On 7/23/2024 4:36 PM, Patrick Robb wrote:
> On Tue, Jul 23, 2024 at 11:31 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>>
>> On 7/22/2024 5:28 PM, Patrick Robb wrote:
>>> On Mon, Jul 22, 2024 at 12:15 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
>>>
>>>>> If we go with the URL option, does is still required to differentiate as
>>>>> "patch-xxx" or "series-yyy", previously they were different IDs, but
>>>>> with URL can patchwork deduce if it is series or patch? If so this can
>>>>> bring a simplification.
>>>>
>>>> No, you can just paste the URL and the Django URL resolver will figure
>>>> out whether it points at a patch or a series. No need to differentiate
>>>> with the URLs.
>>>>
>>>> That's also true of the message ID option too. There isn't much of a
>>>> point in differentiating patch/series message IDs because series do
>>>> not reliably have an email associated with them.
>>>
>>> Sounds good. I want to highlight again for the ci group that all
>>> dependencies will be series dependencies, regardless of whether
>>> "patch-xxx" or "series-yyy" is used. If a patch message id or url is
>>> submitted, it will be mapped to its series url for the dependency.
>>>
>>
>> Are you planning to keep the 'patch' or 'series' part, why not change
>> the syntax as:
>>
>> Depends-on: <message-ID>
>> or
>> Depends-on: <patchwork URL>
>>
> 
> Good point. Yes, there is no reason to keep the "patch" or "series"
> prefix to the value.
> 
>>
>> And is there a benefit to support both "message-ID" and "patchwork URL",
>> so why not just:
>> Depends-on: <patchwork URL>
>>
> 
> Maybe Adam can answer, but I think his intention was to support both
> formats, to provide more flexibility for users.
>

I am not sure if this flexibility is required, I am feeling it can be
simpler to support one.
And parser can convert form one to another if it is required at some
point by the tool.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-23 16:08                     ` Ferruh Yigit
@ 2024-07-24 15:07                       ` Adam Hassick
  2024-07-25  9:47                         ` Ferruh Yigit
  0 siblings, 1 reply; 17+ messages in thread
From: Adam Hassick @ 2024-07-24 15:07 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Patrick Robb, ci, Aaron Conole

On Tue, Jul 23, 2024 at 12:08 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 7/23/2024 4:36 PM, Patrick Robb wrote:
> > On Tue, Jul 23, 2024 at 11:31 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> >>
> >> On 7/22/2024 5:28 PM, Patrick Robb wrote:
> >>> On Mon, Jul 22, 2024 at 12:15 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
> >>>
> >>>>> If we go with the URL option, does is still required to differentiate as
> >>>>> "patch-xxx" or "series-yyy", previously they were different IDs, but
> >>>>> with URL can patchwork deduce if it is series or patch? If so this can
> >>>>> bring a simplification.
> >>>>
> >>>> No, you can just paste the URL and the Django URL resolver will figure
> >>>> out whether it points at a patch or a series. No need to differentiate
> >>>> with the URLs.
> >>>>
> >>>> That's also true of the message ID option too. There isn't much of a
> >>>> point in differentiating patch/series message IDs because series do
> >>>> not reliably have an email associated with them.
> >>>
> >>> Sounds good. I want to highlight again for the ci group that all
> >>> dependencies will be series dependencies, regardless of whether
> >>> "patch-xxx" or "series-yyy" is used. If a patch message id or url is
> >>> submitted, it will be mapped to its series url for the dependency.
> >>>
> >>
> >> Are you planning to keep the 'patch' or 'series' part, why not change
> >> the syntax as:
> >>
> >> Depends-on: <message-ID>
> >> or
> >> Depends-on: <patchwork URL>
> >>
> >
> > Good point. Yes, there is no reason to keep the "patch" or "series"
> > prefix to the value.
> >
> >>
> >> And is there a benefit to support both "message-ID" and "patchwork URL",
> >> so why not just:
> >> Depends-on: <patchwork URL>
> >>
> >
> > Maybe Adam can answer, but I think his intention was to support both
> > formats, to provide more flexibility for users.
> >
>
> I am not sure if this flexibility is required, I am feeling it can be
> simpler to support one.
> And parser can convert form one to another if it is required at some
> point by the tool.

There seems to be some interest in providing multiple types of IDs on
the Patchwork end. Supporting a URL is convenient and supporting a
message ID provides a use case that better aligns with the Patchwork
design principle of not polluting changelogs with Patchwork related
metadata (such as URLs). Also, other SCM tools like Gerrit and b4 use
message ID to reference other patches. I had someone mention adding
the "change-id" feature from b4/gerrit to reference patch series on
the GitHub issue, so there may be interest in adding more accepted
value types to Depends-on in the future. If simplicity for our
developers is the concern, then we could only mention one of these
methods in our documentation.

An unrelated note about the v2: Earlier I mentioned that we might
support cover letter IDs to reference a series dependency. I've
decided to forgo that feature because developers do not always
resubmit cover letters when they submit new versions of a patch
series. For example, if they depend on the v3 of a patch and they use
the cover letter message ID submitted with v1, that will introduce a
dependency on v1. This could lead to some confusion.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-24 15:07                       ` Adam Hassick
@ 2024-07-25  9:47                         ` Ferruh Yigit
  2024-07-25 17:52                           ` Patrick Robb
  0 siblings, 1 reply; 17+ messages in thread
From: Ferruh Yigit @ 2024-07-25  9:47 UTC (permalink / raw)
  To: Adam Hassick; +Cc: Patrick Robb, ci, Aaron Conole

On 7/24/2024 4:07 PM, Adam Hassick wrote:
> On Tue, Jul 23, 2024 at 12:08 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>>
>> On 7/23/2024 4:36 PM, Patrick Robb wrote:
>>> On Tue, Jul 23, 2024 at 11:31 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>>>>
>>>> On 7/22/2024 5:28 PM, Patrick Robb wrote:
>>>>> On Mon, Jul 22, 2024 at 12:15 PM Adam Hassick <ahassick@iol.unh.edu> wrote:
>>>>>
>>>>>>> If we go with the URL option, does is still required to differentiate as
>>>>>>> "patch-xxx" or "series-yyy", previously they were different IDs, but
>>>>>>> with URL can patchwork deduce if it is series or patch? If so this can
>>>>>>> bring a simplification.
>>>>>>
>>>>>> No, you can just paste the URL and the Django URL resolver will figure
>>>>>> out whether it points at a patch or a series. No need to differentiate
>>>>>> with the URLs.
>>>>>>
>>>>>> That's also true of the message ID option too. There isn't much of a
>>>>>> point in differentiating patch/series message IDs because series do
>>>>>> not reliably have an email associated with them.
>>>>>
>>>>> Sounds good. I want to highlight again for the ci group that all
>>>>> dependencies will be series dependencies, regardless of whether
>>>>> "patch-xxx" or "series-yyy" is used. If a patch message id or url is
>>>>> submitted, it will be mapped to its series url for the dependency.
>>>>>
>>>>
>>>> Are you planning to keep the 'patch' or 'series' part, why not change
>>>> the syntax as:
>>>>
>>>> Depends-on: <message-ID>
>>>> or
>>>> Depends-on: <patchwork URL>
>>>>
>>>
>>> Good point. Yes, there is no reason to keep the "patch" or "series"
>>> prefix to the value.
>>>
>>>>
>>>> And is there a benefit to support both "message-ID" and "patchwork URL",
>>>> so why not just:
>>>> Depends-on: <patchwork URL>
>>>>
>>>
>>> Maybe Adam can answer, but I think his intention was to support both
>>> formats, to provide more flexibility for users.
>>>
>>
>> I am not sure if this flexibility is required, I am feeling it can be
>> simpler to support one.
>> And parser can convert form one to another if it is required at some
>> point by the tool.
> 
> There seems to be some interest in providing multiple types of IDs on
> the Patchwork end. Supporting a URL is convenient and supporting a
> message ID provides a use case that better aligns with the Patchwork
> design principle of not polluting changelogs with Patchwork related
> metadata (such as URLs). Also, other SCM tools like Gerrit and b4 use
> message ID to reference other patches. I had someone mention adding
> the "change-id" feature from b4/gerrit to reference patch series on
> the GitHub issue, so there may be interest in adding more accepted
> value types to Depends-on in the future. If simplicity for our
> developers is the concern, then we could only mention one of these
> methods in our documentation.
> 

If there are usecases to use both format, sure. And we can expose all to
users.


> An unrelated note about the v2: Earlier I mentioned that we might
> support cover letter IDs to reference a series dependency. I've
> decided to forgo that feature because developers do not always
> resubmit cover letters when they submit new versions of a patch
> series. For example, if they depend on the v3 of a patch and they use
> the cover letter message ID submitted with v1, that will introduce a
> dependency on v1. This could lead to some confusion.
>

As far as I can see if a patch series has cover letter, more likely it
will have cover letter in next version.

If cover letter is not supported, and dependent series has a cover
letter, is the expectation to add first patch of the series as
dependency skipping cover letter? This maybe also confusing.

For the sample you give, if dependent patch decided to drop cover letter
in v3, won't it work to provide first patch of v3 as dependency?


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Adding Series Dependency to Patchwork
  2024-07-25  9:47                         ` Ferruh Yigit
@ 2024-07-25 17:52                           ` Patrick Robb
  0 siblings, 0 replies; 17+ messages in thread
From: Patrick Robb @ 2024-07-25 17:52 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Adam Hassick, ci, Aaron Conole

On Thu, Jul 25, 2024 at 5:57 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 7/24/2024 4:07 PM, Adam Hassick wrote:
> > On Tue, Jul 23, 2024 at 12:08 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> > An unrelated note about the v2: Earlier I mentioned that we might
> > support cover letter IDs to reference a series dependency. I've
> > decided to forgo that feature because developers do not always
> > resubmit cover letters when they submit new versions of a patch
> > series. For example, if they depend on the v3 of a patch and they use
> > the cover letter message ID submitted with v1, that will introduce a
> > dependency on v1. This could lead to some confusion.
> >
>
> As far as I can see if a patch series has cover letter, more likely it
> will have cover letter in next version.

Okay, this was not my impression, so thanks for setting the record straight.

Sorry Adam for me giving you the wrong idea about common cover letter
resubmission practices. :)

>
> If cover letter is not supported, and dependent series has a cover
> letter, is the expectation to add first patch of the series as
> dependency skipping cover letter? This maybe also confusing.

Yes this would work, but I can see how having this expectation may be
confusing. So, we can abandon this idea.

>
> For the sample you give, if dependent patch decided to drop cover letter
> in v3, won't it work to provide first patch of v3 as dependency?
>

Yes.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-07-25 17:53 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-22 19:51 Adding Series Dependency to Patchwork Adam Hassick
2024-04-04 13:50 ` Patrick Robb
2024-04-05  6:41   ` Thomas Monjalon
2024-04-11 14:18     ` Aaron Conole
2024-04-11 14:33       ` Patrick Robb
2024-07-12 20:15         ` Adam Hassick
2024-07-19 15:32           ` Patrick Robb
2024-07-19 17:43             ` Ferruh Yigit
2024-07-19 17:40           ` Ferruh Yigit
2024-07-22 16:16             ` Adam Hassick
2024-07-22 16:28               ` Patrick Robb
2024-07-23 12:41                 ` Aaron Conole
     [not found]                 ` <6b425d90-78b2-497b-958c-9d36e2ba6e3b@amd.com>
2024-07-23 15:36                   ` Patrick Robb
2024-07-23 16:08                     ` Ferruh Yigit
2024-07-24 15:07                       ` Adam Hassick
2024-07-25  9:47                         ` Ferruh Yigit
2024-07-25 17:52                           ` Patrick Robb

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).