Hi everyone, Updates: GitHub fixed this to preserve the authors information. However, the "committed" field will be "GitHub <nore...@github.com>" instead who merges PR. I reported this in GitHub Community [1] and will track the problem later. Not sure it is a GitHub bug or my setting problem.
Best, Jark [1]: https://github.community/t5/How-to-use-Git-and-GitHub/Authorship-of-merge-commits-made-by-Github-Apps-changed/td-p/48797/highlight/false/page/3 On Sat, 7 Mar 2020 at 16:56, Hequn Cheng <he...@apache.org> wrote: > Hi, > > Thank you all for the discussion! > > On one hand, due to the network problem, the "Squash and merge" button is > very helpful. I’m also getting more and more rely on it as it is also very > convenient. > > On the other hand, I think the concerns raised by Stephan are valid and we > should pay attention to it, i.e., add PR id and don’t squash everything, > etc. Such changes can never be changed once been checked in. Considering > this, I have updated the committer guide wiki page[1] with some > descriptions about the GitHub web UI and some notices about merging code. > Hope it helps and feel free to add more if you find something has still > been missed. > > Best, > Hequn > > [1] > > https://cwiki.apache.org/confluence/display/FLINK/General+Information+for+Committers > > > On Fri, Mar 6, 2020 at 6:55 PM Stephan Ewen <se...@apache.org> wrote: > > > All right sounds fair. > > Especially that the button helps in case of unstable networks makes > sense. > > > > > > On Fri, Mar 6, 2020 at 11:04 AM Aljoscha Krettek <aljos...@apache.org> > > wrote: > > > > > If there is a noreply email address that could be on purpose. This > > > happens when you configure github to not show your real e-mail address. > > > This also happens when contributors open a PR and don't want to show > > > their real e-mail address. I talked to at least 1 person that had it > set > > > up like this on purpose. > > > > > > Best, > > > Aljoscha > > > > > > On 05.03.20 17:37, Stephan Ewen wrote: > > > > It looks like this feature still messes up email addresses, for > example > > > if > > > > you do a "git log | grep noreply" in the repo. > > > > > > > > Don't most PRs consist anyways of multiple commits where we want to > > > > preserve "refactor" and "feature" differentiation in the history, > > rather > > > > than squash everything? > > > > > > > > On Thu, Mar 5, 2020 at 4:54 PM Piotr Nowojski <pi...@ververica.com> > > > wrote: > > > > > > > >> Hi, > > > >> > > > >> If it’s really not preserving ownership (I didn’t notice the problem > > > >> before), +1 for removing “squash and merge”. > > > >> > > > >> However -1 for removing “rebase and merge”. I didn’t see any issues > > with > > > >> it and I’m using it constantly. > > > >> > > > >> Piotrek > > > >> > > > >>> On 5 Mar 2020, at 16:40, Jark Wu <imj...@gmail.com> wrote: > > > >>> > > > >>> Hi all, > > > >>> > > > >>> Thanks for the feedbacks. But I want to clarify the motivation to > > > disable > > > >>> "Squash and merge" is just because of the regression/bug of the > > missing > > > >>> author information. > > > >>> If GitHub fixes this later, I think it makes sense to bring this > > button > > > >>> back. > > > >>> > > > >>> Hi Stephan & Zhijiang, > > > >>> > > > >>> To be honest, I love the "Squash and merge" button and often use > it. > > It > > > >>> saves me a lot of time to merge PRs, because pulling and pushing > > > commits > > > >> in > > > >>> China is very unstable. > > > >>> > > > >>> I don't think the potential problems you mentioned is a "problem". > > > >>> For "Squash and merge", > > > >>> - "Merge commits": there is no "merge" commits, because GitHub will > > > >> squash > > > >>> commits and rebase the commit and then add to the master branch. > > > >>> - "This closes #<pr>" line to track back: when you click "Squash > and > > > >>> merge", it allows you to edit the title and description, so you can > > > >>> add "This closes #<pr>" message to the description the same with in > > the > > > >>> local git. Besides, GitHub automatically append "(#<pr>)" after the > > > >> title, > > > >>> which is also helpful to track. > > > >>> > > > >>> Best, > > > >>> Jark > > > >>> > > > >>> On Thu, 5 Mar 2020 at 23:36, Robert Metzger <rmetz...@apache.org> > > > wrote: > > > >>> > > > >>>> +1 for disabling this feature for now. > > > >>>> > > > >>>> Thanks a lot for spotting this! > > > >>>> > > > >>>> On Thu, Mar 5, 2020 at 3:54 PM Zhijiang < > wangzhijiang...@aliyun.com > > > >>>> .invalid> > > > >>>> wrote: > > > >>>> > > > >>>>> +1 for disabling "Squash and merge" if feasible to do that. > > > >>>>> > > > >>>>> The possible benefit to use this button is for saving some > efforts > > to > > > >>>>> squash some intermediate "[fixup]" commits during PR review. > > > >>>>> But it would bring more potential problems as mentioned below, > > > missing > > > >>>>> author information and message of "This closes #<pr>", etc. > > > >>>>> Even it might cause unexpected format of long commit content > > > >> description > > > >>>>> if not handled carefully in the text box. > > > >>>>> > > > >>>>> Best, > > > >>>>> Zhijiang > > > >>>>> > > > >>>>> > > > >>>>> > ------------------------------------------------------------------ > > > >>>>> From:tison <wander4...@gmail.com> > > > >>>>> Send Time:2020 Mar. 5 (Thu.) 21:34 > > > >>>>> To:dev <dev@flink.apache.org> > > > >>>>> Subject:Re: [DISCUSS] Disable "Squash and merge" button for Flink > > > >>>>> repository on GitHub > > > >>>>> > > > >>>>> Hi Yadong, > > > >>>>> > > > >>>>> Maybe we firstly reach out INFRA team and see the reply from > their > > > >> side. > > > >>>>> > > > >>>>> Since the actual operator is INFRA team, in the dev mailing list > we > > > can > > > >>>>> focus on motivation and > > > >>>>> wait for the reply. > > > >>>>> > > > >>>>> Best, > > > >>>>> tison. > > > >>>>> > > > >>>>> > > > >>>>> Yadong Xie <vthink...@gmail.com> 于2020年3月5日周四 下午9:29写道： > > > >>>>> > > > >>>>>> Hi Jark > > > >>>>>> > > > >>>>>> I think GitHub UI can not disable both the "Squash and merge" > > button > > > >>>> and > > > >>>>>> "Rebase and merge" at the same time if there exists any > protected > > > >>>> branch > > > >>>>> in > > > >>>>>> the repository(according to github rules). > > > >>>>>> > > > >>>>>> If we only left "merge and commits" button, it will against > > > requiring > > > >> a > > > >>>>>> linear commit history rules here > > > >>>>>> > > > >>>>>> > > > >>>>> > > > >>>> > > > >> > > > > > > https://help.github.com/en/github/administering-a-repository/requiring-a-linear-commit-history > > > >>>>>> > > > >>>>>> tison <wander4...@gmail.com> 于2020年3月5日周四 下午9:04写道： > > > >>>>>> > > > >>>>>>> For implement it, file a JIRA ticket in INFRA [1] > > > >>>>>>> > > > >>>>>>> Best, > > > >>>>>>> tison. > > > >>>>>>> [1] https://issues.apache.org/jira/projects/INFRA > > > >>>>>>> > > > >>>>>>> > > > >>>>>>> Stephan Ewen <se...@apache.org> 于2020年3月5日周四 下午8:57写道： > > > >>>>>>> > > > >>>>>>>> Big +1 to disable it. > > > >>>>>>>> > > > >>>>>>>> I have never been a fan, it has always caused problems: > > > >>>>>>>> - Merge commits > > > >>>>>>>> - weird alias emails > > > >>>>>>>> - lost author information > > > >>>>>>>> - commit message misses the "This closes #<pr>" line to > track > > > >>>> back > > > >>>>>>>> commits to PRs/reviews. > > > >>>>>>>> > > > >>>>>>>> The button goes against best practice, it should go away. > > > >>>>>>>> > > > >>>>>>>> Best, > > > >>>>>>>> Stephan > > > >>>>>>>> > > > >>>>>>>> > > > >>>>>>>> On Thu, Mar 5, 2020 at 1:51 PM Yadong Xie < > vthink...@gmail.com> > > > >>>>> wrote: > > > >>>>>>>> > > > >>>>>>>>> Hi Jark > > > >>>>>>>>> There is a conversation about this here: > > > >>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>> > > > >>>> > > > >> > > > > > > https://github.community/t5/How-to-use-Git-and-GitHub/Authorship-of-merge-commits-made-by-Github-Apps-changed/td-p/48797 > > > >>>>>>>>> I think GitHub will fix it soon, it is a bug, not a feature > :). > > > >>>>>>>>> > > > >>>>>>>>> Jingsong Li <jingsongl...@gmail.com> 于2020年3月5日周四 下午8:32写道： > > > >>>>>>>>> > > > >>>>>>>>>> Thanks for deep investigation. > > > >>>>>>>>>> > > > >>>>>>>>>> +1 to disable "Squash and merge" button now. > > > >>>>>>>>>> But I think this is a very serious problem, It affects too > > many > > > >>>>>>> GitHub > > > >>>>>>>>>> workers. Github should deal with it quickly? > > > >>>>>>>>>> > > > >>>>>>>>>> Best, > > > >>>>>>>>>> Jingsong Lee > > > >>>>>>>>>> > > > >>>>>>>>>> On Thu, Mar 5, 2020 at 7:21 PM Xingbo Huang < > > > >>>> hxbks...@gmail.com> > > > >>>>>>>> wrote: > > > >>>>>>>>>> > > > >>>>>>>>>>> Hi Jark, > > > >>>>>>>>>>> > > > >>>>>>>>>>> Thanks for bringing up this discussion. Good catch. Agree > > > >>>> that > > > >>>>> we > > > >>>>>>> can > > > >>>>>>>>>>> disable "Squash and merge"(also the other buttons) for now. > > > >>>>>>>>>>> > > > >>>>>>>>>>> There is a guideline on how to do that in > > > >>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>> > > > >>>> > > > >> > > > > > > https://help.github.com/en/github/administering-a-repository/configuring-commit-squashing-for-pull-requests > > > >>>>>>>>>>> . > > > >>>>>>>>>>> > > > >>>>>>>>>>> Best, > > > >>>>>>>>>>> Xingbo > > > >>>>>>>>>>> > > > >>>>>>>>>>> Jark Wu <imj...@gmail.com> 于2020年3月5日周四 下午6:42写道： > > > >>>>>>>>>>> > > > >>>>>>>>>>>> Hi everyone, > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> We just noticed that everytime a pull request gets merged > > > >>>>> with > > > >>>>>>> the > > > >>>>>>>>>>> "Squash > > > >>>>>>>>>>>> and merge" button, > > > >>>>>>>>>>>> GitHub drops the original authorship information and > > > >>>> changes > > > >>>>>>>>> "authored" > > > >>>>>>>>>>> to > > > >>>>>>>>>>>> whoever merged the PR. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> We found this happened in #11102 [1] and #11302 [2]. It > > > >>>> seems > > > >>>>>>> that > > > >>>>>>>> it > > > >>>>>>>>>> is > > > >>>>>>>>>>> a > > > >>>>>>>>>>>> long outstanding issue > > > >>>>>>>>>>>> and GitHub is aware of it but doesn't make an attempt to > > > >>>> fix > > > >>>>> it > > > >>>>>>>>> [3][4]. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Before this behavior, "authored" is the original author > and > > > >>>>>>>>>> "committed" > > > >>>>>>>>>>> is > > > >>>>>>>>>>>> the one who merged the PR, > > > >>>>>>>>>>>> which was pretty good to record the contributor's > > > >>>>> contribution > > > >>>>>>> and > > > >>>>>>>>> the > > > >>>>>>>>>>>> committed information. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> From the perspective of contributors, it’s really > > > >>>> frustrated > > > >>>>> if > > > >>>>>>>> their > > > >>>>>>>>>>>> authorship information gets lost. > > > >>>>>>>>>>>> Considering we don't know when GitHub will fix it, I > > > >>>> propose > > > >>>>> to > > > >>>>>>>>> disable > > > >>>>>>>>>>>> "Squash and merge" button > > > >>>>>>>>>>>> (and also "Rebase and merge" button) before it is fixed. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> However, I'm not sure how to disable it. Can it be > disabled > > > >>>>> by > > > >>>>>>>> GitHub > > > >>>>>>>>>> UI > > > >>>>>>>>>>> if > > > >>>>>>>>>>>> who has administrator permission? > > > >>>>>>>>>>>> Or .asf.yaml [5] is the right way? > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> What do you think? > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Best, > > > >>>>>>>>>>>> Jark > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> [1]: https://github.com/apache/flink/pull/11102 > > > >>>>>>>>>>>> [2]: https://github.com/apache/flink/pull/11302 > > > >>>>>>>>>>>> [3]: > > > >>>>>>>>>> > > > >>>>> > https://github.com/chdsbd/kodiak/issues/300#issuecomment-595016815 > > > >>>>>>>>>>>> [4]: https://github.com/isaacs/github/issues/1750 > > > >>>>>>>>>>>> [5]: > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>> > > > >>>> > > > >> > > > > > > https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-Mergebuttons > > > >>>>>>>>>>>> > > > >>>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> > > > >>>>>>>>>> -- > > > >>>>>>>>>> Best, Jingsong Lee > > > >>>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>> > > > >>>>> > > > >>>> > > > >> > > > >> > > > > > > > > > >
