[apparmor] [administrivia] git conversion complete; gitlab projects set up

[Christian Boltz]

Hello,

Am Mittwoch, 1. November 2017, 21:46:17 CET schrieb Tyler Hicks:
> On 11/01/2017 02:41 PM, Christian Boltz wrote:

> > Another question is if we want to continue sending patches to the
> > mailinglist, or if we'll switch over to using branches (prefixed
> > with the username, for example "cboltz-utils-foo") and then send
> > merge requests.
>
> What's the reason for the username prefix?

I should probably explain or just show the full workflow we use for 
managing the salt code ;-)

    git checkout -b cboltz-whatever  # create a new branch
    vi $whatever
    git add $whatever
    git commit
    git push  # fails because the branch doesn't have an origin set yet, 
              # but gives copy&paste-ready instructions with the correct 
              # push command
    git push --options-shown-above  # this prints a link for opening a
              # merge request
    # open the given link to create the merge request, and make sure to
    # enable the option to delete the source branch after merging to 
    # avoid having lots of superfluous branches around.

Note that we do all this in the *main repo* (for AppArmor, that would be 
gitlab.com/apparmor/apparmor), not in a user repo like 
gitlab.com/cboltz/apparmor.

That should also explain the reason for the username prefix in the branch 
name - it avoids naming conflicts. (If we decide to do all merge requests 
from gitlab.com/$user/apparmor, the name prefix is superfluous.)

Advantages of having those temporary branches in the main repo are:
- you don't need to refresh your $user repo regularly (by pulling from
  the gitlab.com/apparmor/apparmor repo), just run   git pull
- it's easy to checkout a branch someone created for review

The only disadvantage I see is that having everything as branches in the 
main repo might be a bit noisy, but that's not a real problem.
 
> I really enjoyed the web-based merge request workflow and think it can
> be an improvement over the mailing list patchset based flow. However,
> I'd strongly recommend that we require contributors to:
> 
> 1) Create a merge request
> 2) Receive feedback from maintainers
> 3) If changes are required, fold changes necessary to address feedback
> into the existing patches, rebase, and force push to their merge
> request branch.
> 
> #3 is necessary to avoid a bunch of fixup patches that shouldn't be
> standalone. It also makes for an bisect-able tree since there's no
> broken commits being merged (with separate fixup commits).

Agreed, force push makes sense in this case.

> > I also wonder how to handle the Acked-by messages in case we use
> > merge requests - while it's possible with git rebase + using the
> > "reword" keyword, it means we'll have to force-push to those
> > branches before merging them.
> 
> What the maintainer did for the GitHub contribution that I mentioned
> above was to merge my pull request into a local branch, interactive
> rebase to add his Signed-off-by, and then push the resulting branch to
> to the master branch on GitHub.

As Steve already mentioned, that sounds like lots of work.

IMHO having the Acked-by only in the merge commit would be enough, at 
least if it's the same for all patches/commits in the to-be-merged 
branch.

Things are more interesting[tm] if I ack the first 3 patches of a series, 
and you ack the other 4 - in this (probably rare) case, adjusting the 
commit messages might indeed make sense.


Regards,

Christian Boltz
-- 
sigmonster ist gassi...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20171102/e472cd6b/attachment.sig>