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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> wrote: >>> >>>> +1 for disabling this feature for now. >>>> >>>> Thanks a lot for spotting this! >>>> >>>> On Thu, Mar 5, 2020 at 3:54 PM Zhijiang <[hidden email] >>>> .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 <[hidden email]> >>>>> Send Time:2020 Mar. 5 (Thu.) 21:34 >>>>> To:dev <[hidden email]> >>>>> 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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> >>>>> 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 <[hidden email]> 于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 < >>>> [hidden email]> >>>>>>>> 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 <[hidden email]> 于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 >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>> >> >> > |
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 <[hidden email]> 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 <[hidden email]> > 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 <[hidden email]> 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 <[hidden email]> > wrote: > >>> > >>>> +1 for disabling this feature for now. > >>>> > >>>> Thanks a lot for spotting this! > >>>> > >>>> On Thu, Mar 5, 2020 at 3:54 PM Zhijiang <[hidden email] > >>>> .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 <[hidden email]> > >>>>> Send Time:2020 Mar. 5 (Thu.) 21:34 > >>>>> To:dev <[hidden email]> > >>>>> 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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> > >>>>> 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 <[hidden email]> 于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 < > >>>> [hidden email]> > >>>>>>>> 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 <[hidden email]> 于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 > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>> > >> > >> > > > |
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 <[hidden email]> 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 <[hidden email]> > 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 <[hidden email]> > > 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 <[hidden email]> 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 <[hidden email]> > > wrote: > > >>> > > >>>> +1 for disabling this feature for now. > > >>>> > > >>>> Thanks a lot for spotting this! > > >>>> > > >>>> On Thu, Mar 5, 2020 at 3:54 PM Zhijiang <[hidden email] > > >>>> .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 <[hidden email]> > > >>>>> Send Time:2020 Mar. 5 (Thu.) 21:34 > > >>>>> To:dev <[hidden email]> > > >>>>> 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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> > > >>>>> 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 <[hidden email]> 于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 < > > >>>> [hidden email]> > > >>>>>>>> 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 <[hidden email]> 于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 > > >>>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>>> > > >>>>>> > > >>>>> > > >>>>> > > >>>> > > >> > > >> > > > > > > |
Hi everyone,
Updates: GitHub fixed this to preserve the authors information. However, the "committed" field will be "GitHub <[hidden email]>" 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> > > 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 <[hidden email]> > > > 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 <[hidden email]> 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 <[hidden email]> > > > wrote: > > > >>> > > > >>>> +1 for disabling this feature for now. > > > >>>> > > > >>>> Thanks a lot for spotting this! > > > >>>> > > > >>>> On Thu, Mar 5, 2020 at 3:54 PM Zhijiang < > [hidden email] > > > >>>> .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 <[hidden email]> > > > >>>>> Send Time:2020 Mar. 5 (Thu.) 21:34 > > > >>>>> To:dev <[hidden email]> > > > >>>>> 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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> 于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 < > [hidden email]> > > > >>>>> 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 <[hidden email]> 于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 < > > > >>>> [hidden email]> > > > >>>>>>>> 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 <[hidden email]> 于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 > > > >>>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>> > > > >>>>> > > > >>>> > > > >> > > > >> > > > > > > > > > > |
Hi all,
GitHub Community announced that they have resolved the problem of "squash and merge with [hidden email]" [1]. However, I found this problem still existed if we click "squash and merge" button to merge other's PR. I tested on my private Github repo and found if choose "rebase and merge", the information of author and committer both are right. If choose "squash and merge", the information of author is right while the information of committer would change to [hidden email]. [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 Best Yun Tang ________________________________ From: Jark Wu <[hidden email]> Sent: Monday, March 9, 2020 16:02 To: dev <[hidden email]> Subject: Re: [DISCUSS] Disable "Squash and merge" button for Flink repository on GitHub Hi everyone, Updates: GitHub fixed this to preserve the authors information. However, the "committed" field will be "GitHub <[hidden email]>" 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> > > 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 <[hidden email]> > > > 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 <[hidden email]> 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 <[hidden email]> > > > wrote: > > > >>> > > > >>>> +1 for disabling this feature for now. > > > >>>> > > > >>>> Thanks a lot for spotting this! > > > >>>> > > > >>>> On Thu, Mar 5, 2020 at 3:54 PM Zhijiang < > [hidden email] > > > >>>> .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 <[hidden email]> > > > >>>>> Send Time:2020 Mar. 5 (Thu.) 21:34 > > > >>>>> To:dev <[hidden email]> > > > >>>>> 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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> 于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 < > [hidden email]> > > > >>>>> 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 <[hidden email]> 于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 < > > > >>>> [hidden email]> > > > >>>>>>>> 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 <[hidden email]> 于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 > > > >>>>>>>>>> > > > >>>>>>>>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>>> > > > >>>>> > > > >>>>> > > > >>>> > > > >> > > > >> > > > > > > > > > > |
Free forum by Nabble | Edit this page |