You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shardingsphere.apache.org by 吴晟 Sheng Wu <wu...@foxmail.com> on 2018/11/23 04:07:24 UTC

Re: Recommand to open `Squash and merge` as default merge inGitHubrepo

I am not sure whether `rebase and merge` to avoid scenario(1) or not. `make commit log more clear` is what we try our best to do.


Also remind again, please change the mail name setting, to let others know who is talking :)


------------------
Sheng Wu
Apache SkyWalking & Sharding-Sphere


 




------------------ Original ------------------
From:  "cherrylzhao"<zh...@126.com>;
Date:  Fri, Nov 23, 2018 11:59 AM
To:  "dev"<de...@shardingsphere.apache.org>;

Subject:  Re: Recommand to open `Squash and merge` as default merge inGitHubrepo



Thanks a lot :)

I have exactly understood what keep `squash` in default `commit merge` is available mean.
Also we should use rebase to make commit log more clear and reliable.


> On Nov 23, 2018, at 11:11 AM, 吴晟 Sheng Wu <wu...@foxmail.com> wrote:
> 
> I have no doubt our initial committers could do this well. But I suggest this because of new contributors. Let's think in this scenarios
> 
> 
> 1. A new contributor, and new to GitHub. We checkout his fork and send pull request. But he didn't bind this GitHub mail to his local git. Then if this PR merged, you will lose the visible way to track his contributions. GitHub will not show him in the contributor list.
> 2. A pull request is complex in some ways, and got reviewed/change requests by many times, so the PR author change codes a lot of times. Then how should we merge this PR? Ask him to rebase, or we don't merge? Or merge this by bring several dozens times commits, which will make our revalue more different.
> 
> 
> Both of these came from SkyWalking's experience, so, as soon as ShardingSphere joined the Incubator, I am sharing this for you. Also, you should know, this is the common ways in many Open Source projects. 
> 
> 
> ------------------
> Sheng Wu
> Apache SkyWalking & Sharding-Sphere
> 
> 
> 
> 
> 
> 
> 
> ------------------ Original ------------------
> From:  "cherrylzhao"<zh...@126.com>;
> Date:  Fri, Nov 23, 2018 09:24 AM
> To:  "dev"<de...@shardingsphere.apache.org>;
> 
> Subject:  Re: Recommand to open `Squash and merge` as default merge in GitHubrepo
> 
> 
> 
> In fact, we also use dev branch to add new feature like GitHub issue #1238.
> Usually we make issue id related with commit comment. `squash and merge` maybe not suitable for this scenario.
> I think we should make commit much smaller and organized by functions or modification purpose is better like Willem said.
> Also we can rebase the commit count if the content is not clear.
> 
> On 2018/11/22 10:38:43, "z...@apache.org" <z....@apache.org> wrote: 
>> Yes, right now ShardingShpere always use `dev` branch, and other branch> 
>> just for temporary proposal. We can try to use `squash and merge`.> 
>> 
>> ------------------> 
>> 
>> Liang Zhang (John)> 
>> Apache Sharding-Sphere & Dubbo> 
>> 
>> 
>> Sheng Wu <wu...@apache.org> 于2018年11月22日周四 下午3:19写道:> 
>> 
>>>> But if we have multiple branches and we need to cherry pick the patch> 
>>>> between two different branch. It could be better if the commits is> 
>>>> much smaller and organized by the functions or the modification> 
>>>> purposes.> 
>>>> 
>>> Agree. But just fit for multiple branches in maintaining. So keep `squash`> 
>>> in default, `commit merge` available is a good choice.> 
>>>> 
>>> For ShardingSphere, I don't see that is happening.> 
>>>> 
>>> Sheng Wu.> 
>>>> 
>>> On 2018/11/22 03:21:33, Willem Jiang <wi...@gmail.com> wrote:> 
>>>> If the commits are only for fixing the PR review, I think 'sqash and> 
>>>> merge' is good way to go.> 
>>>> But if we have multiple branches and we need to cherry pick the patch> 
>>>> between two different branch. It could be better if the commits is> 
>>>> much smaller and organized by the functions or the modification> 
>>>> purposes.> 
>>>>> 
>>>> Willem Jiang> 
>>>>> 
>>>> Twitter: willemjiang> 
>>>> Weibo: 姜宁willem> 
>>>>> 
>>>> On Wed, Nov 21, 2018 at 11:02 PM 吴晟 Sheng Wu <wu...@foxmail.com>> 
>>> wrote:> 
>>>>>> 
>>>>> Hi, initial committer> 
>>>>>> 
>>>>>> 
>>>>> I have known, we are going to move the repos. I think we could expect> 
>>> ShardingSphere will have more and more contributors from out of initial> 
>>> committer team. In our Apache releases, we need to provide CHANGELOGS, ref> 
>>> SkyWalking's[1]. We should make commit logs matching the issues and> 
>>> changelogs.> 
>>>>>> 
>>>>>> 
>>>>> Also, in the future, we need to evaluate new committer and PPMC,> 
>>> `squash and merge`makes your commits[2] list more reliable. Because in my> 
>>> personal experiences, some one will submit a lot of commits in PR. Others> 
>>> will rebase, especially the one has more open source experiences. Make> 
>>> commits at least equal the number of PR merged.> 
>>>>>> 
>>>>>> 
>>>>> Of course, this is only my suggestion.> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>> [1] https://github.com/apache/incubator-skywalking/blob/5.x/CHANGES.md> 
>>>>> [2]> 
>>> https://github.com/sharding-sphere/sharding-sphere/graphs/contributors> 
>>>>>> 
>>>>>> 
>>>>> ------------------> 
>>>>> Sheng Wu> 
>>>>> Apache SkyWalking & Sharding-Sphere> 
>>>>> 
>>>>

Re: Recommand to open `Squash and merge` as default merge inGitHubrepo

Posted by "zhangliang@apache.org" <zh...@apache.org>.
How about separate commit action between initial committers and new
committers?
For initial committers, let them use `git rebase -i` to management their
commits; for new committers we just use `squash and merge`.
When new committers has more experience of ShardingSphere, we can let them
to management their commit themselves.

------------------

Liang Zhang (John)
Apache Sharding-Sphere & Dubbo


zhaojun <zh...@126.com> 于2018年11月23日周五 下午1:01写道:

> I have changed mail setting :)
>
> Let us try to use `Squash and merge` & `Rebase and merge` , we’d like to
> share the experience each other.
>
>
> ------------------
> Zhao Jun
> Apache Sharding-Sphere & ServiceComb
>
>
> > On Nov 23, 2018, at 12:07 PM, 吴晟 Sheng Wu <wu...@foxmail.com> wrote:
> >
> > I am not sure whether `rebase and merge` to avoid scenario(1) or not.
> `make commit log more clear` is what we try our best to do.
> >
> >
> > Also remind again, please change the mail name setting, to let others
> know who is talking :)
> >
> >
> > ------------------
> > Sheng Wu
> > Apache SkyWalking & Sharding-Sphere
> >
> >
> >
> >
> >
> >
> >
> > ------------------ Original ------------------
> > From:  "cherrylzhao"<zh...@126.com>;
> > Date:  Fri, Nov 23, 2018 11:59 AM
> > To:  "dev"<de...@shardingsphere.apache.org>;
> >
> > Subject:  Re: Recommand to open `Squash and merge` as default merge
> inGitHubrepo
> >
> >
> >
> > Thanks a lot :)
> >
> > I have exactly understood what keep `squash` in default `commit merge`
> is available mean.
> > Also we should use rebase to make commit log more clear and reliable.
> >
> >
> >> On Nov 23, 2018, at 11:11 AM, 吴晟 Sheng Wu <wu...@foxmail.com> wrote:
> >>
> >> I have no doubt our initial committers could do this well. But I
> suggest this because of new contributors. Let's think in this scenarios
> >>
> >>
> >> 1. A new contributor, and new to GitHub. We checkout his fork and send
> pull request. But he didn't bind this GitHub mail to his local git. Then if
> this PR merged, you will lose the visible way to track his contributions.
> GitHub will not show him in the contributor list.
> >> 2. A pull request is complex in some ways, and got reviewed/change
> requests by many times, so the PR author change codes a lot of times. Then
> how should we merge this PR? Ask him to rebase, or we don't merge? Or merge
> this by bring several dozens times commits, which will make our revalue
> more different.
> >>
> >>
> >> Both of these came from SkyWalking's experience, so, as soon as
> ShardingSphere joined the Incubator, I am sharing this for you. Also, you
> should know, this is the common ways in many Open Source projects.
> >>
> >>
> >> ------------------
> >> Sheng Wu
> >> Apache SkyWalking & Sharding-Sphere
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >> ------------------ Original ------------------
> >> From:  "cherrylzhao"<zh...@126.com>;
> >> Date:  Fri, Nov 23, 2018 09:24 AM
> >> To:  "dev"<de...@shardingsphere.apache.org>;
> >>
> >> Subject:  Re: Recommand to open `Squash and merge` as default merge in
> GitHubrepo
> >>
> >>
> >>
> >> In fact, we also use dev branch to add new feature like GitHub issue
> #1238.
> >> Usually we make issue id related with commit comment. `squash and
> merge` maybe not suitable for this scenario.
> >> I think we should make commit much smaller and organized by functions
> or modification purpose is better like Willem said.
> >> Also we can rebase the commit count if the content is not clear.
> >>
> >> On 2018/11/22 10:38:43, "z...@apache.org" <z....@apache.org> wrote:
> >>> Yes, right now ShardingShpere always use `dev` branch, and other
> branch>
> >>> just for temporary proposal. We can try to use `squash and merge`.>
> >>>
> >>> ------------------>
> >>>
> >>> Liang Zhang (John)>
> >>> Apache Sharding-Sphere & Dubbo>
> >>>
> >>>
> >>> Sheng Wu <wu...@apache.org> 于2018年11月22日周四 下午3:19写道:>
> >>>
> >>>>> But if we have multiple branches and we need to cherry pick the
> patch>
> >>>>> between two different branch. It could be better if the commits is>
> >>>>> much smaller and organized by the functions or the modification>
> >>>>> purposes.>
> >>>>>
> >>>> Agree. But just fit for multiple branches in maintaining. So keep
> `squash`>
> >>>> in default, `commit merge` available is a good choice.>
> >>>>>
> >>>> For ShardingSphere, I don't see that is happening.>
> >>>>>
> >>>> Sheng Wu.>
> >>>>>
> >>>> On 2018/11/22 03:21:33, Willem Jiang <wi...@gmail.com> wrote:>
> >>>>> If the commits are only for fixing the PR review, I think 'sqash
> and>
> >>>>> merge' is good way to go.>
> >>>>> But if we have multiple branches and we need to cherry pick the
> patch>
> >>>>> between two different branch. It could be better if the commits is>
> >>>>> much smaller and organized by the functions or the modification>
> >>>>> purposes.>
> >>>>>>
> >>>>> Willem Jiang>
> >>>>>>
> >>>>> Twitter: willemjiang>
> >>>>> Weibo: 姜宁willem>
> >>>>>>
> >>>>> On Wed, Nov 21, 2018 at 11:02 PM 吴晟 Sheng Wu <wu...@foxmail.com>>
> >>>> wrote:>
> >>>>>>>
> >>>>>> Hi, initial committer>
> >>>>>>>
> >>>>>>>
> >>>>>> I have known, we are going to move the repos. I think we could
> expect>
> >>>> ShardingSphere will have more and more contributors from out of
> initial>
> >>>> committer team. In our Apache releases, we need to provide
> CHANGELOGS, ref>
> >>>> SkyWalking's[1]. We should make commit logs matching the issues and>
> >>>> changelogs.>
> >>>>>>>
> >>>>>>>
> >>>>>> Also, in the future, we need to evaluate new committer and PPMC,>
> >>>> `squash and merge`makes your commits[2] list more reliable. Because
> in my>
> >>>> personal experiences, some one will submit a lot of commits in PR.
> Others>
> >>>> will rebase, especially the one has more open source experiences.
> Make>
> >>>> commits at least equal the number of PR merged.>
> >>>>>>>
> >>>>>>>
> >>>>>> Of course, this is only my suggestion.>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>> [1]
> https://github.com/apache/incubator-skywalking/blob/5.x/CHANGES.md>
> >>>>>> [2]>
> >>>>
> https://github.com/sharding-sphere/sharding-sphere/graphs/contributors>
> >>>>>>>
> >>>>>>>
> >>>>>> ------------------>
> >>>>>> Sheng Wu>
> >>>>>> Apache SkyWalking & Sharding-Sphere>
> >>>>>>
>
>

Re: Recommand to open `Squash and merge` as default merge inGitHubrepo

Posted by zhaojun <zh...@126.com>.
I have changed mail setting :)

Let us try to use `Squash and merge` & `Rebase and merge` , we’d like to share the experience each other.


------------------
Zhao Jun
Apache Sharding-Sphere & ServiceComb


> On Nov 23, 2018, at 12:07 PM, 吴晟 Sheng Wu <wu...@foxmail.com> wrote:
> 
> I am not sure whether `rebase and merge` to avoid scenario(1) or not. `make commit log more clear` is what we try our best to do.
> 
> 
> Also remind again, please change the mail name setting, to let others know who is talking :)
> 
> 
> ------------------
> Sheng Wu
> Apache SkyWalking & Sharding-Sphere
> 
> 
> 
> 
> 
> 
> 
> ------------------ Original ------------------
> From:  "cherrylzhao"<zh...@126.com>;
> Date:  Fri, Nov 23, 2018 11:59 AM
> To:  "dev"<de...@shardingsphere.apache.org>;
> 
> Subject:  Re: Recommand to open `Squash and merge` as default merge inGitHubrepo
> 
> 
> 
> Thanks a lot :)
> 
> I have exactly understood what keep `squash` in default `commit merge` is available mean.
> Also we should use rebase to make commit log more clear and reliable.
> 
> 
>> On Nov 23, 2018, at 11:11 AM, 吴晟 Sheng Wu <wu...@foxmail.com> wrote:
>> 
>> I have no doubt our initial committers could do this well. But I suggest this because of new contributors. Let's think in this scenarios
>> 
>> 
>> 1. A new contributor, and new to GitHub. We checkout his fork and send pull request. But he didn't bind this GitHub mail to his local git. Then if this PR merged, you will lose the visible way to track his contributions. GitHub will not show him in the contributor list.
>> 2. A pull request is complex in some ways, and got reviewed/change requests by many times, so the PR author change codes a lot of times. Then how should we merge this PR? Ask him to rebase, or we don't merge? Or merge this by bring several dozens times commits, which will make our revalue more different.
>> 
>> 
>> Both of these came from SkyWalking's experience, so, as soon as ShardingSphere joined the Incubator, I am sharing this for you. Also, you should know, this is the common ways in many Open Source projects. 
>> 
>> 
>> ------------------
>> Sheng Wu
>> Apache SkyWalking & Sharding-Sphere
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> ------------------ Original ------------------
>> From:  "cherrylzhao"<zh...@126.com>;
>> Date:  Fri, Nov 23, 2018 09:24 AM
>> To:  "dev"<de...@shardingsphere.apache.org>;
>> 
>> Subject:  Re: Recommand to open `Squash and merge` as default merge in GitHubrepo
>> 
>> 
>> 
>> In fact, we also use dev branch to add new feature like GitHub issue #1238.
>> Usually we make issue id related with commit comment. `squash and merge` maybe not suitable for this scenario.
>> I think we should make commit much smaller and organized by functions or modification purpose is better like Willem said.
>> Also we can rebase the commit count if the content is not clear.
>> 
>> On 2018/11/22 10:38:43, "z...@apache.org" <z....@apache.org> wrote: 
>>> Yes, right now ShardingShpere always use `dev` branch, and other branch> 
>>> just for temporary proposal. We can try to use `squash and merge`.> 
>>> 
>>> ------------------> 
>>> 
>>> Liang Zhang (John)> 
>>> Apache Sharding-Sphere & Dubbo> 
>>> 
>>> 
>>> Sheng Wu <wu...@apache.org> 于2018年11月22日周四 下午3:19写道:> 
>>> 
>>>>> But if we have multiple branches and we need to cherry pick the patch> 
>>>>> between two different branch. It could be better if the commits is> 
>>>>> much smaller and organized by the functions or the modification> 
>>>>> purposes.> 
>>>>> 
>>>> Agree. But just fit for multiple branches in maintaining. So keep `squash`> 
>>>> in default, `commit merge` available is a good choice.> 
>>>>> 
>>>> For ShardingSphere, I don't see that is happening.> 
>>>>> 
>>>> Sheng Wu.> 
>>>>> 
>>>> On 2018/11/22 03:21:33, Willem Jiang <wi...@gmail.com> wrote:> 
>>>>> If the commits are only for fixing the PR review, I think 'sqash and> 
>>>>> merge' is good way to go.> 
>>>>> But if we have multiple branches and we need to cherry pick the patch> 
>>>>> between two different branch. It could be better if the commits is> 
>>>>> much smaller and organized by the functions or the modification> 
>>>>> purposes.> 
>>>>>> 
>>>>> Willem Jiang> 
>>>>>> 
>>>>> Twitter: willemjiang> 
>>>>> Weibo: 姜宁willem> 
>>>>>> 
>>>>> On Wed, Nov 21, 2018 at 11:02 PM 吴晟 Sheng Wu <wu...@foxmail.com>> 
>>>> wrote:> 
>>>>>>> 
>>>>>> Hi, initial committer> 
>>>>>>> 
>>>>>>> 
>>>>>> I have known, we are going to move the repos. I think we could expect> 
>>>> ShardingSphere will have more and more contributors from out of initial> 
>>>> committer team. In our Apache releases, we need to provide CHANGELOGS, ref> 
>>>> SkyWalking's[1]. We should make commit logs matching the issues and> 
>>>> changelogs.> 
>>>>>>> 
>>>>>>> 
>>>>>> Also, in the future, we need to evaluate new committer and PPMC,> 
>>>> `squash and merge`makes your commits[2] list more reliable. Because in my> 
>>>> personal experiences, some one will submit a lot of commits in PR. Others> 
>>>> will rebase, especially the one has more open source experiences. Make> 
>>>> commits at least equal the number of PR merged.> 
>>>>>>> 
>>>>>>> 
>>>>>> Of course, this is only my suggestion.> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>> [1] https://github.com/apache/incubator-skywalking/blob/5.x/CHANGES.md> 
>>>>>> [2]> 
>>>> https://github.com/sharding-sphere/sharding-sphere/graphs/contributors> 
>>>>>>> 
>>>>>>> 
>>>>>> ------------------> 
>>>>>> Sheng Wu> 
>>>>>> Apache SkyWalking & Sharding-Sphere> 
>>>>>>