On 11/02/2017 02:19 PM, Tyler Hicks wrote: > On 11/02/2017 04:08 PM, John Johansen wrote: >> On 11/02/2017 01:03 PM, Tyler Hicks wrote: >>> On 11/02/2017 03:00 PM, John Johansen wrote: >>>> ] >>>>> We walked through a merge yesterday with this merge request: >>>>> >>>>> https://gitlab.com/apparmor/apparmor/merge_requests/1 >>>>> >>>>> The audit trail of who merged the code is implicitly present in the >>>>> merge commit. By default, there's no information about who reviewed the >>>>> changes but the merge commit contains a mention of where to find the >>>>> merge request and that page will contain much more info about who >>>>> reviewed which parts of the merge request. >>>>> >>>> That makes the dangerous assumption we keep our infrastructure on gitlab, >>>> and don't endup migrating again (this is the 4 or 5 migration in the >>>> projects history). I would strongly prefer having that information >>>> integral to the commit message. >>>> >>>>> I'm fine with the default merge commit message. I think Steve had an >>>>> issue with the subject line of the default merge commit message. I'll >>>>> let him voice his opposition to it and maybe he'll have a better >>>>> suggestion. >>>>> I am really not happy with with what I have seen so far. >>>> >>>> Merge branch 'make-variable' into 'master' >>>> >>>> all: Use the MAKE variable >>>> >>>> See merge request apparmor/apparmor!1 >>>> >>>> >>>> uhmm, no that really fails the migration test >>> >>> Please provide a suggested commit message format that we can all follow. >>> >> I don't have one, I am really not too bothered with the format. I don't >> even care if people are consistent with it. >> >> What is missing is a couple critical pieces of information. Who was >> involved in the merge discussion doing both the reviewing and acking. >> Partly as breadcrumbs in the future partly because I want a more >> permanent form of acknowledgement of the contribution, which is >> both critical and all too easily overlooked. >> >> I don't even care if it shows up on every patch in the merge or just >> the merge message. I just want the info available in the logs, >> instead of in the meta info stored in the cloud that may one day >> disappear. > > Does anyone else have strong feelings about this and care to suggest a > format/process? We're seemingly blocked on accepting merge requests > otherwise. >
So I am not a fan of the merge commit, it creates a messier history and can break bisecting, especially where its an interaction between certain patches you are looking for not just a single breaking commit. But I can live with it, with some provisos. The person who does the merge commit, show up as the author of the merge, I'm not thrilled with that. As he may just be the person doing the signed-off-by: for the merge (and yes I know the individual commits within retain the proper authorship). So if we use the web merge, make sure to edit the commit message. Add the necessary reviewer and acked-by lines. And I can live with this. Requiring people to do this locally via a rebase and editing each commit feels like too much of a barrier. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
