Patch Review Flow

Patch Review Flow

[jgart]

Hi Guixers,

Does anyone follow this workflow for reviewing patches?

git clone https://git.guix-patches.cbaines.net/guix-patches/
git checkout issue-xxxxx
git format-patch ...
# then in the development checkout of Guix:
git am ...; make; ./pre-inst-env guix build

Should we document it in the manual?

Does anyone follow a workflow that is more efficient than the above?

all best,

jgart

Re: Patch Review Flow

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]


"jgart" <jgart@dismail.de> writes:

> Does anyone follow this workflow for reviewing patches?
>
> git clone https://git.guix-patches.cbaines.net/guix-patches/
> git checkout issue-xxxxx
> git format-patch ...
> # then in the development checkout of Guix:
> git am ...; make; ./pre-inst-env guix build
>
> Should we document it in the manual?
>
> Does anyone follow a workflow that is more efficient than the above?

I have the guix-patches repository as a remote, which I then cherry pick
from (via Magit), which makes it quick and easy to apply patches.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: Patch Review Flow

[Clément Lassieur]

On Mon, Nov 06 2023, Christopher Baines wrote:

> "jgart" <jgart@dismail.de> writes:
>
>> Does anyone follow this workflow for reviewing patches?
>>
>> git clone https://git.guix-patches.cbaines.net/guix-patches/
>> git checkout issue-xxxxx
>> git format-patch ...
>> # then in the development checkout of Guix:
>> git am ...; make; ./pre-inst-env guix build
>>
>> Should we document it in the manual?
>>
>> Does anyone follow a workflow that is more efficient than the above?
>
> I have the guix-patches repository as a remote, which I then cherry pick
> from (via Magit), which makes it quick and easy to apply patches.

Or with Emacs you can apply patches directly from the Gnus or the
Debbugs interface.

Re: Patch Review Flow

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 1137 bytes --]

On Mon, Nov 06, 2023 at 07:51:58AM +0000, Christopher Baines wrote:
> 
> "jgart" <jgart@dismail.de> writes:
> 
> > Does anyone follow this workflow for reviewing patches?
> >
> > git clone https://git.guix-patches.cbaines.net/guix-patches/
> > git checkout issue-xxxxx
> > git format-patch ...
> > # then in the development checkout of Guix:
> > git am ...; make; ./pre-inst-env guix build
> >
> > Should we document it in the manual?
> >
> > Does anyone follow a workflow that is more efficient than the above?
> 
> I have the guix-patches repository as a remote, which I then cherry pick
> from (via Magit), which makes it quick and easy to apply patches.

I also have the guix-patches repository as a remote, and then I use tig
to look at the different branches there.  I haven't checked to see if
it's possible to use git am or git cherry-pick from tig's interface so
I'm doing that one by one.


-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Patch Review Flow

[jgart]

Hi Efraim,

Thanks for sharing your approach as well. IIRC, a user can't interactively cherry-pick a range with tig. If you find a way please do share ;()

Re: Patch Review Flow

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]

On Tue, Nov 07, 2023 at 12:32:35PM +0000, jgart wrote:
> Hi Efraim,
> 
> Thanks for sharing your approach as well. IIRC, a user can't interactively cherry-pick a range with tig. If you find a way please do share ;()

It looks like 'C' (that's a 'big c') over a commit will offer to
cherry-pick the commit.  Alternatively, if you're still looking at the
branches then 'C' will offer to checkout that branch.

Tig asks for an explicit y or n, so pressing enter for the prompt won't
do anything.

I should probably look at the different options tig has. I've had it
installed forever but only ever used it for viewing commits and
branches.

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Patch Review Flow

[jgart]

> I've had it installed forever but only ever used it for viewing commits and branches.

ditto!

--
jgart

Re: Patch Review Flow

[jgart]

> which I then cherry pick from (via Magit), which makes it quick and easy to apply patches.

Hi Chris,

That's a great idea to use magit to select the range of patches to apply. 

I was doing it in a more painful way via the CLI.

thanks,

jgart

Re: Patch Review Flow

[Maxim Cournoyer]

Hi jgart,

"jgart" <jgart@dismail.de> writes:

> Hi Guixers,
>
> Does anyone follow this workflow for reviewing patches?
>
> git clone https://git.guix-patches.cbaines.net/guix-patches/
> git checkout issue-xxxxx
> git format-patch ...
> # then in the development checkout of Guix:
> git am ...; make; ./pre-inst-env guix build
>
> Should we document it in the manual?

There are many flows... perhaps we should illustrate them via examples
in the Guix Cookbook manual?

> Does anyone follow a workflow that is more efficient than the above?

You can use the 'b4' tool to apply a series via a message's Message-ID
that is from that series.  Or you can use Emacs-Debbugs patch buffer
(which is powered by Gnus), ensure your current directory for the main
buffer is set to your Guix checkout (e.g. 'M-x cd RET ~/src/guix'), then
apply the patches pressing the '|' to pipe it to 'git am -3' command
with it.

-- 
Thanks,
Maxim

Re: Patch Review Flow

[jgart]

There's also the following workflow:

Click on an individual patch in the mumi web interface to download it to ~/Downloads.

cd guix-checkout
git am -S ~/Downloads/53019-76.mbox

Go back to the mumi interface and click on the next patch. Repeat. 😹😹😹

> Should we document an approach in the manual?

Yes.

Re: Patch Review Flow

[Tobias Alexandra Platen]

This is also interesting for me, since I have created my first patch
for guix: adding libsurvive as a package.

Alex