workflow

workflow

[Fabio Aiuto]

Good morning all,

I need some clarifications about the workflow.

As a newcomer in kernel development and open source development
in general I got a little used to with the forkflow in Greg's staging
subsystem.

The tool I'm used to work with is, at the moment, command line git
against Greg's tree.

I tried to apply the same approach on rust-for-linux tree but I noticed
some differences, first of all the use of GitHub.

One misterious to me side effect is that the command `git show` on HEAD
pointing to a PR doesn't show diffs.

After these two enlightening informal meetings I realized that, for this
particular project, I must (and I'm happy to do it) align with you all
using GitHub. Is there a particular join procedure you recommend?

thank you,

fabio

Re: workflow

[Geert Stappers]

On Sun, May 09, 2021 at 09:33:38AM +0200, Fabio Aiuto wrote:
> Good morning all,

Hail all time zones,

 
> I need some clarifications about the workflow.
> 
> As a newcomer in kernel development and open source development
> in general I got a little used to with the forkflow in Greg's staging
> subsystem.
> 
> The tool I'm used to work with is, at the moment, command line git
> against Greg's tree.
> 
> I tried to apply the same approach on rust-for-linux tree but I noticed
> some differences, first of all the use of GitHub.
> 
> One misterious to me side effect is that the command `git show` on HEAD
> pointing to a PR doesn't show diffs.
> 
> After these two enlightening informal meetings I realized that, for this
> particular project, I must (and I'm happy to do it) align with you all
> using GitHub. Is there a particular join procedure you recommend?

I think original poster
is not aware of "Rust for Linux" already in the next branch.


 
> thank you,

My appology for this way too short message.

 
> fabio


Groeten
Geert Stappers
-- 
Silence is hard to parse

Re: workflow

[Fabio Aiuto]

On Sun, May 09, 2021 at 11:31:50AM +0200, Geert Stappers wrote:
> On Sun, May 09, 2021 at 09:33:38AM +0200, Fabio Aiuto wrote:
> > Good morning all,
> 
> Hail all time zones,
> 
>  
> > I need some clarifications about the workflow.
> > 
> > As a newcomer in kernel development and open source development
> > in general I got a little used to with the forkflow in Greg's staging
> > subsystem.
> > 
> > The tool I'm used to work with is, at the moment, command line git
> > against Greg's tree.
> > 
> > I tried to apply the same approach on rust-for-linux tree but I noticed
> > some differences, first of all the use of GitHub.
> > 
> > One misterious to me side effect is that the command `git show` on HEAD
> > pointing to a PR doesn't show diffs.
> > 
> > After these two enlightening informal meetings I realized that, for this
> > particular project, I must (and I'm happy to do it) align with you all
> > using GitHub. Is there a particular join procedure you recommend?
> 
> I think original poster
> is not aware of "Rust for Linux" already in the next branch.

ok I will search on Google what does it means, for sure it's more
friendly even if it requires an account

> 
> 
>  
> > thank you,
> 
> My appology for this way too short message.
> 
>  
> > fabio
> 
> 
> Groeten
> Geert Stappers
> -- 
> Silence is hard to parse

thank you,

fabio

Re: workflow

[Miguel Ojeda]

Hi Fabio,

On Sun, May 9, 2021 at 9:34 AM Fabio Aiuto <fabioaiuto83@gmail.com> wrote:
>
> One misterious to me side effect is that the command `git show` on HEAD
> pointing to a PR doesn't show diffs.

That is because they are merge commits (i.e. a commit with more than
one parent). `git show` may show a "combined diff" in such a case. For
instance, it does right now with the current commit `rust` is pointing
to, i.e. commit 99bec9d3476d ("Merge pull request #227 from
Kloenk/ra").

See e.g. https://stackoverflow.com/questions/40986518/git-show-of-a-merge-commit
for a detailed explanation.

But you are 100% right! The tree does not look like other ones in the
kernel. If you take a look into the other branch (the one I manually
prepare, `rust-next`), it looks like you would expect (and does not
contain GitHub files etc.).

In order to fix this (plus a few other benefits), we will have a
better workflow soon (as soon as I get some time to prepare it, say a
few weeks).

> After these two enlightening informal meetings I realized that, for this
> particular project, I must (and I'm happy to do it) align with you all

It is not a hard requirement -- you can send patches to this mailing
list and I will gladly take them!

The technical benefits of using GitHub for us are the issue tracker
and the CI, mainly.

> using GitHub. Is there a particular join procedure you recommend?

Using GitHub if you already know Git and the kernel workflow is very easy:

  - `git push` your changes to your "forked repo" in GitHub.

  - Create a pull request in the web interface.

That should be pretty much it.

Cheers,
Miguel

Re: workflow

[Fabio Aiuto]

On Sun, May 09, 2021 at 02:13:13PM +0200, Miguel Ojeda wrote:
> Hi Fabio,
> 
> On Sun, May 9, 2021 at 9:34 AM Fabio Aiuto <fabioaiuto83@gmail.com> wrote:
> >
> > One misterious to me side effect is that the command `git show` on HEAD
> > pointing to a PR doesn't show diffs.
> 
> That is because they are merge commits (i.e. a commit with more than
> one parent). `git show` may show a "combined diff" in such a case. For
> instance, it does right now with the current commit `rust` is pointing
> to, i.e. commit 99bec9d3476d ("Merge pull request #227 from
> Kloenk/ra").
> 
> See e.g. https://stackoverflow.com/questions/40986518/git-show-of-a-merge-commit
> for a detailed explanation.
> 
> But you are 100% right! The tree does not look like other ones in the
> kernel. If you take a look into the other branch (the one I manually
> prepare, `rust-next`), it looks like you would expect (and does not
> contain GitHub files etc.).
> 
> In order to fix this (plus a few other benefits), we will have a
> better workflow soon (as soon as I get some time to prepare it, say a
> few weeks).
> 
> > After these two enlightening informal meetings I realized that, for this
> > particular project, I must (and I'm happy to do it) align with you all
> 
> It is not a hard requirement -- you can send patches to this mailing
> list and I will gladly take them!
> 
> The technical benefits of using GitHub for us are the issue tracker
> and the CI, mainly.
> 
> > using GitHub. Is there a particular join procedure you recommend?
> 
> Using GitHub if you already know Git and the kernel workflow is very easy:
> 
>   - `git push` your changes to your "forked repo" in GitHub.
> 
>   - Create a pull request in the web interface.
> 
> That should be pretty much it.
> 
> Cheers,
> Miguel

thank you so much Miguel, that's all I wanted to know :)

fabio

Re: Workflow

[Bradley M. Kuhn]

Thomas Gleixner wrote:
> I appreciate the +1, but I would appreciate even more some hint what you
> think is a digestible set size and how much time you think it takes to
> review such a set.

I think these questions are hard to answer until we try some.  License
provenance review is very different than code review.  I suggest we just try
it in the batch-size you suggest and then be willing to readjust based on
everyone's feedback after doing it for a while.

--
Bradley M. Kuhn

Pls. support the charity where I work, Software Freedom Conservancy:
https://sfconservancy.org/supporter/

Re: Workflow

[Kate Stewart]

On Thu, May 9, 2019 at 4:56 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Kate,
>
> On Thu, 9 May 2019, Kate Stewart wrote:
> > On Thu, May 9, 2019 at 9:11 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> > > Any opinions on the size of the batches and how long it will take to get
> > > the reviews done or any other suggestions for a workable solution?
> >
> > Grouping them in digestible sets, and organizing each patch around a
> > pattern makes sense to me.     +1
>
> I appreciate the +1, but I would appreciate even more some hint what you
> think is a digestible set size and how much time you think it takes to
> review such a set. You know that this is just a question of time.

Heh,  fair enough.    I meant I am fine with the proposal of 10-25 a
day you outlined.
Biasing more to the 25 side is ok - sooner we're through the bulk of
them the better.  :-)

Thanks, Kate

Re: Workflow

[Allison Randal]

On 5/10/19 11:54 AM, Thomas Gleixner wrote:
> 
> I'm keeping the first few file diffs in the mail anyway, I just need to
> cutoff for the larger patches to avoid the size limit of the list. The URL
> is there for reference and for those who really want to look at every
> change :)

Makes sense. That'll be perfect for me. :)

Allison

Re: Workflow

[Thomas Gleixner]

Allison,

On Fri, 10 May 2019, Allison Randal wrote:

> On 5/9/19 10:56 PM, Thomas Gleixner wrote:
> > 
> > I appreciate the +1, but I would appreciate even more some hint what you
> > think is a digestible set size and how much time you think it takes to
> > review such a set. You know that this is just a question of time.
> 
> I'm fine with the 10-25/day you suggested, as long as I know to expect
> them. I'll just do a quick run with my morning coffee. Personally, I'm
> also fine if you just want to do all 500+ at once, and I'll work my way
> through them in larger batches (dealing with several thousand emails a
> day is not unusual for me, and these are quick reviews that don't need a
> substantial reply). But, I suspect you're right that smaller batches
> will be easier for most people.
> 
> I will use the git URL to look at the actual changes, because my brain
> is trained to think in diffs, but I'll only review the first few files
> in each diff as a sample.

I'm keeping the first few file diffs in the mail anyway, I just need to
cutoff for the larger patches to avoid the size limit of the list. The URL
is there for reference and for those who really want to look at every
change :)

Thanks,

	tglx

Re: Workflow

[Allison Randal]

On 5/9/19 10:56 PM, Thomas Gleixner wrote:
> 
> I appreciate the +1, but I would appreciate even more some hint what you
> think is a digestible set size and how much time you think it takes to
> review such a set. You know that this is just a question of time.

I'm fine with the 10-25/day you suggested, as long as I know to expect
them. I'll just do a quick run with my morning coffee. Personally, I'm
also fine if you just want to do all 500+ at once, and I'll work my way
through them in larger batches (dealing with several thousand emails a
day is not unusual for me, and these are quick reviews that don't need a
substantial reply). But, I suspect you're right that smaller batches
will be easier for most people.

I will use the git URL to look at the actual changes, because my brain
is trained to think in diffs, but I'll only review the first few files
in each diff as a sample.

Allison

Re: Workflow

[Greg KH]

On Thu, May 09, 2019 at 04:11:38PM +0200, Thomas Gleixner wrote:
> Folks,
> 
> we should ASAP agree on a workflow for going through those 504
> patches. Here is my proposal:
> 
>   1) Use the one patch per identified pattern approach as demonstrated
>      with the first 4.
> 
>   2) Focus the review on the 'pattern -> SPDX id' conclusion
> 
>   3) Trust the automated patcher to do the right thing.

Sounds good to me.

> From my experience with this, it's the most sensible way, as it makes it
> scalable.
> 
> Versus trusting the patcher: I'm surely spending time on looking at the
> actual changes, but that's also massively based on automated analysis which
> exposes only unexpected changes and does not force me to stare at 20k+
> patched instances.
> 
> If we can agree on the above, then I'd like to send out batches of N
> patches, where N would be something in the range of 10-25. These patches
> are basically changelog only because quite some of the patches are too long
> for posting on the list. They surely will contain a git URL so you can look
> at the actual file changes as well (if you are masochistic enough).

I like longer batches, as I'm used to them, but 20-25 is fine.

> Ideally we get a quick feedback (OK/NOK) for each patch in a batch. The OK
> should be preferrably in the form of a 'Reviewed-by: Your Name <your@mail>'
> tag. We'll mention in the changelog that the Review is limited to the
> pattern -> SPDX id conclusion and does not cover the actual file level
> changes. I'll take the blame when then patcher gets it wrong :)
> 
> If a patch is deemed NOK we don't have to sort it out immediately. We can
> post pone it and handle it on the side so the queue is not held up.
> 
> Once a patch has collected Reviewed-by tags we would apply it to a
> repository and send it in batches to Linus.

Sounds reasonable.

> If a batch is consumed (except for the NOK parts) the next batch would be
> posted. Assumed we can handle 10 'pattern -> SPDX id' reviews per day, that
> would take ~10 weeks. Which is quite some time when we want to be halfways
> SPDX clean for the next LTS kernel. So I rather see larger batches
> processed faster :)

You should be able to send multiple batches at a time, right?  But 10
weeks isn't all that bad, I would shoot to get these all into the 5.4
kernel, so we can be done with it :)

> Any opinions on the size of the batches and how long it will take to get
> the reviews done or any other suggestions for a workable solution?

Normally I just wait 2 weeks for tiny patches, or one kernel release
cycle.  If no objections, then I merge to a tree that Linus can pull
from.

As a good example of this, I have some debugfs x86 patches that I posted
a while ago, no maintainer said anything, or took them into their own
tree, so I'll just merge them to a local tree to be sent off for 5.3-rc1 :)

thanks,

greg k-h

Re: Workflow

[Thomas Gleixner]

Kate,

On Thu, 9 May 2019, Kate Stewart wrote:
> On Thu, May 9, 2019 at 9:11 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> > Any opinions on the size of the batches and how long it will take to get
> > the reviews done or any other suggestions for a workable solution?
> 
> Grouping them in digestible sets, and organizing each patch around a
> pattern makes sense to me.     +1

I appreciate the +1, but I would appreciate even more some hint what you
think is a digestible set size and how much time you think it takes to
review such a set. You know that this is just a question of time.

At a set size of one and one week per set it will take 504 weeks ~= 9.3
years from now not having taken the changing code base into account. That's
still slightly faster (by 1 year or so) than waiting for it to be cleaned
up manually by random people. :)

Thanks,

	tglx

Re: Workflow

[Kate Stewart]

Hi Thomas,

On Thu, May 9, 2019 at 9:11 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Folks,
>
> we should ASAP agree on a workflow for going through those 504
> patches. Here is my proposal:
>
>   1) Use the one patch per identified pattern approach as demonstrated
>      with the first 4.
>
>   2) Focus the review on the 'pattern -> SPDX id' conclusion
>
>   3) Trust the automated patcher to do the right thing.
>
> From my experience with this, it's the most sensible way, as it makes it
> scalable.
>
> Versus trusting the patcher: I'm surely spending time on looking at the
> actual changes, but that's also massively based on automated analysis which
> exposes only unexpected changes and does not force me to stare at 20k+
> patched instances.
>
> If we can agree on the above, then I'd like to send out batches of N
> patches, where N would be something in the range of 10-25. These patches
> are basically changelog only because quite some of the patches are too long
> for posting on the list. They surely will contain a git URL so you can look
> at the actual file changes as well (if you are masochistic enough).
>
> Ideally we get a quick feedback (OK/NOK) for each patch in a batch. The OK
> should be preferrably in the form of a 'Reviewed-by: Your Name <your@mail>'
> tag. We'll mention in the changelog that the Review is limited to the
> pattern -> SPDX id conclusion and does not cover the actual file level
> changes. I'll take the blame when then patcher gets it wrong :)
>
> If a patch is deemed NOK we don't have to sort it out immediately. We can
> post pone it and handle it on the side so the queue is not held up.
>
> Once a patch has collected Reviewed-by tags we would apply it to a
> repository and send it in batches to Linus.
>
> If a batch is consumed (except for the NOK parts) the next batch would be
> posted. Assumed we can handle 10 'pattern -> SPDX id' reviews per day, that
> would take ~10 weeks. Which is quite some time when we want to be halfways
> SPDX clean for the next LTS kernel. So I rather see larger batches
> processed faster :)
>
> Any opinions on the size of the batches and how long it will take to get
> the reviews done or any other suggestions for a workable solution?

Grouping them in digestible sets, and organizing each patch around a
pattern makes sense to me.     +1

Kate

Workflow

[Thomas Gleixner]

Folks,

we should ASAP agree on a workflow for going through those 504
patches. Here is my proposal:

  1) Use the one patch per identified pattern approach as demonstrated
     with the first 4.

  2) Focus the review on the 'pattern -> SPDX id' conclusion

  3) Trust the automated patcher to do the right thing.

From my experience with this, it's the most sensible way, as it makes it
scalable.

Versus trusting the patcher: I'm surely spending time on looking at the
actual changes, but that's also massively based on automated analysis which
exposes only unexpected changes and does not force me to stare at 20k+
patched instances.

If we can agree on the above, then I'd like to send out batches of N
patches, where N would be something in the range of 10-25. These patches
are basically changelog only because quite some of the patches are too long
for posting on the list. They surely will contain a git URL so you can look
at the actual file changes as well (if you are masochistic enough).

Ideally we get a quick feedback (OK/NOK) for each patch in a batch. The OK
should be preferrably in the form of a 'Reviewed-by: Your Name <your@mail>'
tag. We'll mention in the changelog that the Review is limited to the
pattern -> SPDX id conclusion and does not cover the actual file level
changes. I'll take the blame when then patcher gets it wrong :)

If a patch is deemed NOK we don't have to sort it out immediately. We can
post pone it and handle it on the side so the queue is not held up.

Once a patch has collected Reviewed-by tags we would apply it to a
repository and send it in batches to Linus.

If a batch is consumed (except for the NOK parts) the next batch would be
posted. Assumed we can handle 10 'pattern -> SPDX id' reviews per day, that
would take ~10 weeks. Which is quite some time when we want to be halfways
SPDX clean for the next LTS kernel. So I rather see larger batches
processed faster :)

Any opinions on the size of the batches and how long it will take to get
the reviews done or any other suggestions for a workable solution?

Thanks,

	tglx