Re: [PATCH qa-frontpage] Apply incoming patches onto the correct feature branch

[Christopher Baines]

Christopher Baines <mail@cbaines.net> writes:

> Vivien Kraus <vivien@planete-kraus.eu> writes:
>
>> It seems that the only available information of the feature branch is
>> in the patch name.
>> ---
>>
>> Hello guix!
>>
>> Thank you for making the QA tool. It seems to me that feature branches
>> are ignored for patches. The patches seem to always be applied on top
>> of master. I get that idea because the base-for-issue-* tag is always
>> put on HEAD, and all my patches targetting gnome-team recently get
>> applied to master. I understand that the latter could be a problem
>> with me.
>>
>> If patches are indeed applied on top of master, then the question
>> arises, what to do. The available information from patchwork is
>> scarce: the base-commit is not available, and the optional feature
>> branch only appears in the name of the patches of the series.
>>
>> I think it is possible to parse the patch names. This way, we get some
>> useful information: the feature branch, the series revision, and the
>> patch index. The patch index can be used to make sure the patches are
>> in correct order (this can be the case if a server in the path
>> re-orders the emails).
>>
>> What do you think?
>
> Hi Vivien,
>
> Thanks for trying to implement this and sending a patch, I think it's a
> good feature to add but there's a few things needed to make this work.
>
> Firstly, you need to actually change which branch the patch is applied
> to, and the place to do that is here [1] in the call to
> with-git-worktree. You can probably use get-commit from
> (guix-qa-frontpage git-repository) to find the commit hash for a
> branch. You'll also probably need to move around some of the code in
> create-branch-for-issue so that you have the data about the patches to
> work out what the desired branch is before you call with-git-worktree.
>
> 1: 
> https://git.cbaines.net/guix/qa-frontpage/tree/guix-qa-frontpage/manage-patch-branches.scm#n250
>
> The second thing is that there's other places in the codebase that
> assume the patches have been applied to the master branch. In
> start-manage-patch-branches-thread, there's
> get-changes-compared-to-master which is used to decide if a branch needs
> recreating if there's too many changes between the base revision and a
> recent master branch revision. That'll need changing to at least skip
> over any patches been applied to a non-master branch, or perform the
> comparison against the tip of the relevant branch.
>
> The third thing that will need to change to allow this to happen is
> adding clear messaging on to the issue page to indicate where patches
> have been applied to non-master branches. That'll help to avoid people
> merging these patches in to the master branch rather than the branch
> they were intended for.
>
> Maybe what would be helpful is a procedure in (guix-qa-frontpage issue)
> that takes the data for the issue from latest-patchwork-series-by-issue,
> and gives you back the branch name that the patches should be applied
> to. That can then be used to get the branch information for all the
> above 3 use cases.
>
> As for trying to order the patches, do you know of a case where the
> ordering from Patchwork is wrong? I think it's only worth changing the
> ordering in the qa-frontpage if we know we're doing it for a reason.
>
> There's now a create-issue-branch command to the guix-qa-frontpage
> script as well, which will be a good way of testing the code out for
> applying patches to various branches. It'll fail when it comes to
> pushing the branch, but it'll still be useful for testing the code up to
> that point.
>
> Would you be able to look at sending some revised patches?

I've now gone ahead and applied a slimmed down version of this patch,
and made the other changes to get a basic form of applying patches to
specific branches in place. There's probably going to be some problems
and more error handling to add, but this should be a good starting
point.

Thanks,

Chris

signature.asc
Description: PGP signature