Re: squashing commits or not
On Sun, Mar 8, 2020 at 5:43 PM Gregory Nutt wrote: > Only slightly related... This morning, I removed some of the writing > instructions from the trop draft work flow proposal. I think they have > served their purpose and destroy readability of the document. I also > delete all of the comments that I could (I can't delete other people's > comments and I can't delete my comments if people responded to them). > > Perhaps this will may the document less intimidating. It would be good > to get all of the other kruft out of the document to so that we can > refine what's there. If other people will delete there own, obsolete > comments, I will delete more of mine as well. I went ahead and deleted my obsolete comments (the ones that don't have replies). I agree that we should remove junk from there, and try to button up the last few missing pieces (marked by red "REVISIT" notes). It would be nice to vote soon on making this our official workflow (at least version 1 of it; it can always be improved in the future). Cheers, Nathan
Re: squashing commits or not
If we go back to my original email on this subject a couple of months ago, I suggested to begin with a tl;dr; section and then follow it with the detailed text. Now that we have the detailed text, it's a simple matter to summarize it and put the summary at the top. Only slightly related... This morning, I removed some of the writing instructions from the trop draft work flow proposal. I think they have served their purpose and destroy readability of the document. I also delete all of the comments that I could (I can't delete other people's comments and I can't delete my comments if people responded to them). Perhaps this will may the document less intimidating. It would be good to get all of the other kruft out of the document to so that we can refine what's there. If other people will delete there own, obsolete comments, I will delete more of mine as well. Greg
Re: squashing commits or not
On Fri, Mar 6, 2020 at 10:20 AM Abdelatif Guettouche < abdelatif.guettou...@gmail.com> wrote: > I think you made it clear that you prefer a TL;DR; document. Maybe we > can have both. If we go back to my original email on this subject a couple of months ago, I suggested to begin with a tl;dr; section and then follow it with the detailed text. Now that we have the detailed text, it's a simple matter to summarize it and put the summary at the top. We will need a second document explaining the release process that we haven't designed yet.
Re: squashing commits or not
I think you made it clear that you prefer a TL;DR; document. Maybe we can have both. But I don't see what best practices that link shows, it's basically a shrinked version of what we already have (minus the submodule stuff). >> ...not hundreds of useless emails saying merged prxyz! >> >> We need to think Team. We need to value the team's time. None of what we >> have done foster communication or provide useful information. There are just >> thousands of emails on a really small project. Can you imagine your inbox >> with 1000 devs working on this? You are referring to the automated emails sent to commit@. You use Github, there is no point for you to get those emails. You can auto archive/delete them or unsubscribe from the commit@ list. That being said, I don't see how this is relevant. Whatever workflow we choose, they are going to be there. What do you think we need to do to foster communication? On Fri, Mar 6, 2020 at 2:11 PM David Sidrane wrote: > > > I see your point, however > > I do not think the overwhelming feeling is real enough for the folks the > contributed to the document. > > Now that we really see a how small (and painful submitting) the patch to PR > ratio can we not focus on the past not "best practices" and document a real > work flow? > > Here is what that 10,404 word document can look like in 490 words. > > https://dev.px4.io/master/en/contribute/git_examples.html#git-examples > > I do not own it, I use it, it works, it informs the developer of what is > happening not hundreds of useless emails saying merged prxyz! > > We need to think Team. We need to value the team's time. None of what we > have done foster communication or provide useful information. There are just > thousands of emails on a really small project. Can you imagine your inbox > with 1000 devs working on this? > > We have some air time on in the Apache world. It is time to renovate! > > David > > > -Original Message- > From: Abdelatif Guettouche [mailto:abdelatif.guettou...@gmail.com] > Sent: Friday, March 06, 2020 3:30 AM > To: dev@nuttx.apache.org > Subject: Re: squashing commits or not > > > Thank you. The link did not lad me and I have no idea what to look at > > there > > among the 10,404 words... > > I see your point, however, the steps (and git commands) are all in one > place with the necessary explanations. If one decides not to read > anything but the commands, they are highlighted with code blocks. > > > That has never been thoroughly approved and is not the accepted workflow > > at present. > > Yes, but eventually, that's the workflow we are going to call a vote on. > All committers can edit and improve it. Non committers can ask to have > the necessary permissions. > > > On Thu, Mar 5, 2020 at 5:55 PM David Sidrane > wrote: > > > > Abdelatif, > > > > Thank you. The link did not lad me and I have no idea what to look at > > there > > among the 10,404 words... > > > > David > > > > -Original Message- > > From: Abdelatif Guettouche [mailto:abdelatif.guettou...@gmail.com] > > Sent: Thursday, March 05, 2020 10:41 AM > > To: dev@nuttx.apache.org > > Subject: Re: squashing commits or not > > > > > How about clear to the point work steps? Do we have the interim workflow > > > listed anywhere that it can be read, without the diatribes? > > > > I just wrote something really quickly. Maybe you want to take a look [1] > > We can add to that section more information on how to squash WIP > > commits using interactive rebasing. > > I'll come back later to do more. But I need to go. Feel free to edit. > > > > 1. > > https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton#CodeContributionWorkflow--BrennanAshton-BeforeSubmittingYourChanges > > > > > > On Thu, Mar 5, 2020 at 5:37 PM David Sidrane > > wrote: > > > > > > I think you are over-reading this. If we do not have a clear list of > > > instructions we are not helping, we are confusing our would be > > > committers > > > (Additional Committers email). If you do not think of them as Rules but > > > more > > > of sharing your "best engineering judgment" to educate the group, you > > > know, > > > Share the knowledge help the community, help the project > > > > > > -Original Message- > > > From: Gregory Nutt [mailto:spudan...@gmail.com] > > > Sent: Thursday, March 05, 2020 9:25 AM > > > To: dev@nuttx.apache.org > > > Subject: R
RE: squashing commits or not
> I see your point, however I do not think the overwhelming feeling is real enough for the folks the contributed to the document. Now that we really see a how small (and painful submitting) the patch to PR ratio can we not focus on the past not "best practices" and document a real work flow? Here is what that 10,404 word document can look like in 490 words. https://dev.px4.io/master/en/contribute/git_examples.html#git-examples I do not own it, I use it, it works, it informs the developer of what is happening not hundreds of useless emails saying merged prxyz! We need to think Team. We need to value the team's time. None of what we have done foster communication or provide useful information. There are just thousands of emails on a really small project. Can you imagine your inbox with 1000 devs working on this? We have some air time on in the Apache world. It is time to renovate! David -Original Message- From: Abdelatif Guettouche [mailto:abdelatif.guettou...@gmail.com] Sent: Friday, March 06, 2020 3:30 AM To: dev@nuttx.apache.org Subject: Re: squashing commits or not > Thank you. The link did not lad me and I have no idea what to look at > there > among the 10,404 words... I see your point, however, the steps (and git commands) are all in one place with the necessary explanations. If one decides not to read anything but the commands, they are highlighted with code blocks. > That has never been thoroughly approved and is not the accepted workflow > at present. Yes, but eventually, that's the workflow we are going to call a vote on. All committers can edit and improve it. Non committers can ask to have the necessary permissions. On Thu, Mar 5, 2020 at 5:55 PM David Sidrane wrote: > > Abdelatif, > > Thank you. The link did not lad me and I have no idea what to look at > there > among the 10,404 words... > > David > > -Original Message- > From: Abdelatif Guettouche [mailto:abdelatif.guettou...@gmail.com] > Sent: Thursday, March 05, 2020 10:41 AM > To: dev@nuttx.apache.org > Subject: Re: squashing commits or not > > > How about clear to the point work steps? Do we have the interim workflow > > listed anywhere that it can be read, without the diatribes? > > I just wrote something really quickly. Maybe you want to take a look [1] > We can add to that section more information on how to squash WIP > commits using interactive rebasing. > I'll come back later to do more. But I need to go. Feel free to edit. > > 1. > https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton#CodeContributionWorkflow--BrennanAshton-BeforeSubmittingYourChanges > > > On Thu, Mar 5, 2020 at 5:37 PM David Sidrane > wrote: > > > > I think you are over-reading this. If we do not have a clear list of > > instructions we are not helping, we are confusing our would be > > committers > > (Additional Committers email). If you do not think of them as Rules but > > more > > of sharing your "best engineering judgment" to educate the group, you > > know, > > Share the knowledge help the community, help the project.... > > > > -Original Message- > > From: Gregory Nutt [mailto:spudan...@gmail.com] > > Sent: Thursday, March 05, 2020 9:25 AM > > To: dev@nuttx.apache.org > > Subject: Re: squashing commits or not > > > > No one gets to set any rules. No one gets to enforce any rules. > > Committers are free to do what they choose. That is the Apache way: It > > is anarchy held together by a belief in common principles and a project > > culture. If you can't trust people to do that job, you are working on > > the wrong project. > > > > I will no be held by any such rules. I will always use my best > > engineering judgement. And I will take my scolding when I deserve it. > > > > What you can do, is help to educate people about the pros and cons of > > the work in general. You have to trust that it is everyone's intention > > in their heart to do the best job that they can. But you will never be > > able to force rules to control others behavior in this environment. No > > one has the authority to do that.
Re: squashing commits or not
> Thank you. The link did not lad me and I have no idea what to look at there > among the 10,404 words... I see your point, however, the steps (and git commands) are all in one place with the necessary explanations. If one decides not to read anything but the commands, they are highlighted with code blocks. > That has never been thoroughly approved and is not the accepted workflow > at present. Yes, but eventually, that's the workflow we are going to call a vote on. All committers can edit and improve it. Non committers can ask to have the necessary permissions. On Thu, Mar 5, 2020 at 5:55 PM David Sidrane wrote: > > Abdelatif, > > Thank you. The link did not lad me and I have no idea what to look at there > among the 10,404 words... > > David > > -Original Message- > From: Abdelatif Guettouche [mailto:abdelatif.guettou...@gmail.com] > Sent: Thursday, March 05, 2020 10:41 AM > To: dev@nuttx.apache.org > Subject: Re: squashing commits or not > > > How about clear to the point work steps? Do we have the interim workflow > > listed anywhere that it can be read, without the diatribes? > > I just wrote something really quickly. Maybe you want to take a look [1] > We can add to that section more information on how to squash WIP > commits using interactive rebasing. > I'll come back later to do more. But I need to go. Feel free to edit. > > 1. > https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton#CodeContributionWorkflow--BrennanAshton-BeforeSubmittingYourChanges > > > On Thu, Mar 5, 2020 at 5:37 PM David Sidrane > wrote: > > > > I think you are over-reading this. If we do not have a clear list of > > instructions we are not helping, we are confusing our would be committers > > (Additional Committers email). If you do not think of them as Rules but > > more > > of sharing your "best engineering judgment" to educate the group, you > > know, > > Share the knowledge help the community, help the project > > > > -----Original Message- > > From: Gregory Nutt [mailto:spudan...@gmail.com] > > Sent: Thursday, March 05, 2020 9:25 AM > > To: dev@nuttx.apache.org > > Subject: Re: squashing commits or not > > > > No one gets to set any rules. No one gets to enforce any rules. > > Committers are free to do what they choose. That is the Apache way: It > > is anarchy held together by a belief in common principles and a project > > culture. If you can't trust people to do that job, you are working on > > the wrong project. > > > > I will no be held by any such rules. I will always use my best > > engineering judgement. And I will take my scolding when I deserve it. > > > > What you can do, is help to educate people about the pros and cons of > > the work in general. You have to trust that it is everyone's intention > > in their heart to do the best job that they can. But you will never be > > able to force rules to control others behavior in this environment. No > > one has the authority to do that.
RE: squashing commits or not
Abdelatif, Thank you. The link did not lad me and I have no idea what to look at there among the 10,404 words... David -Original Message- From: Abdelatif Guettouche [mailto:abdelatif.guettou...@gmail.com] Sent: Thursday, March 05, 2020 10:41 AM To: dev@nuttx.apache.org Subject: Re: squashing commits or not > How about clear to the point work steps? Do we have the interim workflow > listed anywhere that it can be read, without the diatribes? I just wrote something really quickly. Maybe you want to take a look [1] We can add to that section more information on how to squash WIP commits using interactive rebasing. I'll come back later to do more. But I need to go. Feel free to edit. 1. https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton#CodeContributionWorkflow--BrennanAshton-BeforeSubmittingYourChanges On Thu, Mar 5, 2020 at 5:37 PM David Sidrane wrote: > > I think you are over-reading this. If we do not have a clear list of > instructions we are not helping, we are confusing our would be committers > (Additional Committers email). If you do not think of them as Rules but > more > of sharing your "best engineering judgment" to educate the group, you > know, > Share the knowledge help the community, help the project > > -Original Message- > From: Gregory Nutt [mailto:spudan...@gmail.com] > Sent: Thursday, March 05, 2020 9:25 AM > To: dev@nuttx.apache.org > Subject: Re: squashing commits or not > > No one gets to set any rules. No one gets to enforce any rules. > Committers are free to do what they choose. That is the Apache way: It > is anarchy held together by a belief in common principles and a project > culture. If you can't trust people to do that job, you are working on > the wrong project. > > I will no be held by any such rules. I will always use my best > engineering judgement. And I will take my scolding when I deserve it. > > What you can do, is help to educate people about the pros and cons of > the work in general. You have to trust that it is everyone's intention > in their heart to do the best job that they can. But you will never be > able to force rules to control others behavior in this environment. No > one has the authority to do that.
Re: squashing commits or not
I just wrote something really quickly. Maybe you want to take a look [1] We can add to that section more information on how to squash WIP commits using interactive rebasing. I'll come back later to do more. But I need to go. Feel free to edit. 1. https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton#CodeContributionWorkflow--BrennanAshton-BeforeSubmittingYourChanges That has never been thoroughly approved and is not the accepted workflow at present. If people want to get serious about this other than just shooting your mouth off in a public forum, that document plus taking some personal responsibility would be the place to start.
Re: squashing commits or not
Why would you merge it to branch it is not ready? There is no way to know if it is not ready. There is no CI for PRs now. You have to merge first, then you can see if it is ready. Let's not confuse what-I-am-doing-now with the final workflow which is NOT defined, NOT approved, and NOT in effect. There is no CI qualification of PRs, only me doing whatever-the-hell-I-want. That can and will be fixed in the future.
Re: squashing commits or not
> How about clear to the point work steps? Do we have the interim workflow > listed anywhere that it can be read, without the diatribes? I just wrote something really quickly. Maybe you want to take a look [1] We can add to that section more information on how to squash WIP commits using interactive rebasing. I'll come back later to do more. But I need to go. Feel free to edit. 1. https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton#CodeContributionWorkflow--BrennanAshton-BeforeSubmittingYourChanges On Thu, Mar 5, 2020 at 5:37 PM David Sidrane wrote: > > I think you are over-reading this. If we do not have a clear list of > instructions we are not helping, we are confusing our would be committers > (Additional Committers email). If you do not think of them as Rules but more > of sharing your "best engineering judgment" to educate the group, you know, > Share the knowledge help the community, help the project > > -Original Message- > From: Gregory Nutt [mailto:spudan...@gmail.com] > Sent: Thursday, March 05, 2020 9:25 AM > To: dev@nuttx.apache.org > Subject: Re: squashing commits or not > > No one gets to set any rules. No one gets to enforce any rules. > Committers are free to do what they choose. That is the Apache way: It > is anarchy held together by a belief in common principles and a project > culture. If you can't trust people to do that job, you are working on > the wrong project. > > I will no be held by any such rules. I will always use my best > engineering judgement. And I will take my scolding when I deserve it. > > What you can do, is help to educate people about the pros and cons of > the work in general. You have to trust that it is everyone's intention > in their heart to do the best job that they can. But you will never be > able to force rules to control others behavior in this environment. No > one has the authority to do that.
RE: squashing commits or not
Why would you merge it to branch it is not ready? This is not hard. The PR will not pass CI if it fails Style check. Period! That is a simple check. We should have that by now. Also add it to the commit hooks locally. (document the work flow and add the script to download it - or put it in the build make checks it and ask to install it) The problem solved by workflow and tools. -Original Message- From: Gregory Nutt [mailto:spudan...@gmail.com] Sent: Thursday, March 05, 2020 9:32 AM To: dev@nuttx.apache.org Subject: Re: squashing commits or not One big problem that I encounter is that people change .c and .h files but do not run nxstyle against the resulting files. It is not really possible to verify that online (currently, hopefully soon, however). So you don't know about the stylistic problems until you have already merged onto a branch. In that case, your commits WILL be modified. No modifed .c or .h files may come into the repository without passing nxstyle. So, if contributors don't want people mucking with the changes, please run nxstyle before creating the PR. Otherwise, you will have to live with the consequences. We are currently operating under the old workflow of the pre-Apache Bitbucket days. That will transition to the Apache workflow at some point in the future when the tools, triggers, and documentation are in place. They are not now. The old workflow is that I basically do whatever the hell I want to. That can be fixed by getting tools, processes, documentation, and wider participation in place. So currently, if you don't like things, too bad. so sad.
RE: squashing commits or not
I think you are over-reading this. If we do not have a clear list of instructions we are not helping, we are confusing our would be committers (Additional Committers email). If you do not think of them as Rules but more of sharing your "best engineering judgment" to educate the group, you know, Share the knowledge help the community, help the project -Original Message- From: Gregory Nutt [mailto:spudan...@gmail.com] Sent: Thursday, March 05, 2020 9:25 AM To: dev@nuttx.apache.org Subject: Re: squashing commits or not No one gets to set any rules. No one gets to enforce any rules. Committers are free to do what they choose. That is the Apache way: It is anarchy held together by a belief in common principles and a project culture. If you can't trust people to do that job, you are working on the wrong project. I will no be held by any such rules. I will always use my best engineering judgement. And I will take my scolding when I deserve it. What you can do, is help to educate people about the pros and cons of the work in general. You have to trust that it is everyone's intention in their heart to do the best job that they can. But you will never be able to force rules to control others behavior in this environment. No one has the authority to do that.
Re: squashing commits or not
One big problem that I encounter is that people change .c and .h files but do not run nxstyle against the resulting files. It is not really possible to verify that online (currently, hopefully soon, however). So you don't know about the stylistic problems until you have already merged onto a branch. In that case, your commits WILL be modified. No modifed .c or .h files may come into the repository without passing nxstyle. So, if contributors don't want people mucking with the changes, please run nxstyle before creating the PR. Otherwise, you will have to live with the consequences. We are currently operating under the old workflow of the pre-Apache Bitbucket days. That will transition to the Apache workflow at some point in the future when the tools, triggers, and documentation are in place. They are not now. The old workflow is that I basically do whatever the hell I want to. That can be fixed by getting tools, processes, documentation, and wider participation in place. So currently, if you don't like things, too bad. so sad.
Re: squashing commits or not
No one gets to set any rules. No one gets to enforce any rules. Committers are free to do what they choose. That is the Apache way: It is anarchy held together by a belief in common principles and a project culture. If you can't trust people to do that job, you are working on the wrong project. I will no be held by any such rules. I will always use my best engineering judgement. And I will take my scolding when I deserve it. What you can do, is help to educate people about the pros and cons of the work in general. You have to trust that it is everyone's intention in their heart to do the best job that they can. But you will never be able to force rules to control others behavior in this environment. No one has the authority to do that.
RE: squashing commits or not
If you have clear work steps for a rebase work flow (prior to review). Contributors can and will do PR hygiene. -Original Message- From: Gregory Nutt [mailto:spudan...@gmail.com] Sent: Thursday, March 05, 2020 9:20 AM To: dev@nuttx.apache.org Subject: Re: squashing commits or not > If you do have a PR with B1 and B2 commits and generates in master: > > A1 <- B1 <- B2 <- C1 <- master If B1 and B2 are part of the same change, I would probably use git rebase -i to squash only those two
Re: squashing commits or not
If you do have a PR with B1 and B2 commits and generates in master: A1 <- B1 <- B2 <- C1 <- master If B1 and B2 are part of the same change, I would probably use git rebase -i to squash only those two
Re: squashing commits or not
On Thu, Mar 5, 2020 at 5:30 PM David Sidrane wrote: > Isn't that is just more noise? > Noise that you can filter is less noisy. If you do have a PR with B1 and B2 commits and generates in master: A1 <- B1 <- B2 <- C1 <- master you cannot filter it easily. If you generate with branch merges: A <- B <- C <- master ^- B1 <- B2 <-/ then using git log gives: C B B1 B2 A but git log --merges gives: C B A so it is way less noisy but you do not remove from the repo the subcommit information (a.k.a. as noise by the people that don't need/care that information) Anyway, if the common used tool (aka Github web interface) does not provide that option, maybe yes, it is always noise for most people. So I probably agree with you in this case and anyway I will use whatever the community decide to use. > -Original Message- > From: Miguel Ángel Herranz [mailto:mig...@midokura.com.INVALID] > Sent: Thursday, March 05, 2020 8:16 AM > To: dev@nuttx.apache.org > Subject: Re: squashing commits or not > > Why not just leave all the commits and generate on master a single merge > commit? > > In git CLI you can easily filter the non-merge commits using `git log > --merges` (same sequence as squashed PR) and then inspect the involved > commits looking at the non-master commit parent. > > Not sure if the problem is that Github UI is not able to filter this way. > > On Thu, Mar 5, 2020 at 5:00 PM Gregory Nutt wrote: > > > We have to trust that committers will do the best job that they can. We > > have no control over how committers do that job. There is no mechanism > > for such control. This is an area where there is no option but to trust > > the judgement of the committers. > > > > Discussing good practice is fine, but there is no authority to control > > the behavior of the committers. The committers are free to do as they > > choose. The only "punishment" is peer pressure and social acceptance. > > > > > > > > -- > > Miguel Ángel Herranz Trillo > > Software developer > > mig...@midokura.com > > All rights reserved. All logos and descriptions are property of their > respective owners > > > > > > LEGAL NOTICE: This mail and its attached files are confidential and are > exclusively intended to their addresses. In case you may receive this mail > not being its address, we beg you to let us know the error by reply and to > proceed to delete it. The circulation by any mean of this mail could be > penalized in accordance with the applicable legislation. The use of both > the transmitter and the addresses with a commercial aim, or in order to be > incorporated to automated files, is not authorized. > -- Miguel Ángel Herranz Trillo Software developer mig...@midokura.com All rights reserved. All logos and descriptions are property of their respective owners LEGAL NOTICE: This mail and its attached files are confidential and are exclusively intended to their addresses. In case you may receive this mail not being its address, we beg you to let us know the error by reply and to proceed to delete it. The circulation by any mean of this mail could be penalized in accordance with the applicable legislation. The use of both the transmitter and the addresses with a commercial aim, or in order to be incorporated to automated files, is not authorized.
RE: squashing commits or not
How about clear to the point work steps? Do we have the interim workflow listed anywhere that it can be read, without the diatribes? -Original Message- From: Gregory Nutt [mailto:spudan...@gmail.com] Sent: Thursday, March 05, 2020 8:00 AM To: dev@nuttx.apache.org Subject: Re: squashing commits or not We have to trust that committers will do the best job that they can. We have no control over how committers do that job. There is no mechanism for such control. This is an area where there is no option but to trust the judgement of the committers. Discussing good practice is fine, but there is no authority to control the behavior of the committers. The committers are free to do as they choose. The only "punishment" is peer pressure and social acceptance.
RE: squashing commits or not
Isn't that is just more noise? -Original Message- From: Miguel Ángel Herranz [mailto:mig...@midokura.com.INVALID] Sent: Thursday, March 05, 2020 8:16 AM To: dev@nuttx.apache.org Subject: Re: squashing commits or not Why not just leave all the commits and generate on master a single merge commit? In git CLI you can easily filter the non-merge commits using `git log --merges` (same sequence as squashed PR) and then inspect the involved commits looking at the non-master commit parent. Not sure if the problem is that Github UI is not able to filter this way. On Thu, Mar 5, 2020 at 5:00 PM Gregory Nutt wrote: > We have to trust that committers will do the best job that they can. We > have no control over how committers do that job. There is no mechanism > for such control. This is an area where there is no option but to trust > the judgement of the committers. > > Discussing good practice is fine, but there is no authority to control > the behavior of the committers. The committers are free to do as they > choose. The only "punishment" is peer pressure and social acceptance. > > > -- Miguel Ángel Herranz Trillo Software developer mig...@midokura.com All rights reserved. All logos and descriptions are property of their respective owners LEGAL NOTICE: This mail and its attached files are confidential and are exclusively intended to their addresses. In case you may receive this mail not being its address, we beg you to let us know the error by reply and to proceed to delete it. The circulation by any mean of this mail could be penalized in accordance with the applicable legislation. The use of both the transmitter and the addresses with a commercial aim, or in order to be incorporated to automated files, is not authorized.
Re: squashing commits or not
Why not just leave all the commits and generate on master a single merge commit? In git CLI you can easily filter the non-merge commits using `git log --merges` (same sequence as squashed PR) and then inspect the involved commits looking at the non-master commit parent. Not sure if the problem is that Github UI is not able to filter this way. On Thu, Mar 5, 2020 at 5:00 PM Gregory Nutt wrote: > We have to trust that committers will do the best job that they can. We > have no control over how committers do that job. There is no mechanism > for such control. This is an area where there is no option but to trust > the judgement of the committers. > > Discussing good practice is fine, but there is no authority to control > the behavior of the committers. The committers are free to do as they > choose. The only "punishment" is peer pressure and social acceptance. > > > -- Miguel Ángel Herranz Trillo Software developer mig...@midokura.com All rights reserved. All logos and descriptions are property of their respective owners LEGAL NOTICE: This mail and its attached files are confidential and are exclusively intended to their addresses. In case you may receive this mail not being its address, we beg you to let us know the error by reply and to proceed to delete it. The circulation by any mean of this mail could be penalized in accordance with the applicable legislation. The use of both the transmitter and the addresses with a commercial aim, or in order to be incorporated to automated files, is not authorized.
Re: squashing commits or not
We have to trust that committers will do the best job that they can. We have no control over how committers do that job. There is no mechanism for such control. This is an area where there is no option but to trust the judgement of the committers. Discussing good practice is fine, but there is no authority to control the behavior of the committers. The committers are free to do as they choose. The only "punishment" is peer pressure and social acceptance.
Re: squashing commits or not
On Thu, Mar 5, 2020 at 10:22 PM Gregory Nutt wrote: > > The decision to squash or not will be left to the best judgement of the > committer. Yes, committer could give the suggestion, or even decline the PR, but it isn't good practice to directly do squash without any discussion. Even worse, committer may do some change in the squash process and then introduce the potential build break or runtime bug. Our precheck system is almost ready now: https://github.com/apache/incubator-nuttx/pull/261 https://github.com/apache/incubator-nuttx-apps/pull/113 But the precheck can just catch the error in PR. If maintainer make any change locally and commit it directly to the mainline, the whole precheck system just become another Maginot Line.
Re: squashing commits or not
The decision to squash or not will be left to the best judgement of the committer.
Re: squashing commits or not
On Thu, Mar 5, 2020 at 7:58 PM Abdelatif Guettouche wrote: > > > Because if > > the contributor take the time to split the change into serveral small > > patch, which mean it's valuable to do so. > > I agree, but this isn't always the case. > It is okay to have PRs with multiple commits, as you mentioned, we can > even request to do so. > However, some of the commits are just a sequence of the same change. > That's valuable for a WIP, but IMO should've been squashed before > creating the PR. Yes, the contributor should make sure the patch set is split/merged in a reasonable way before creating PR. If not, maintainter could discuss the issue in the PR and let contributor update PR again.This is better than maintainer directly squash the patch set into mainline, because we need teach the new member what's the best practice the community like to follow and avoid they make the same mistake in the new PR. > > > On Thu, Mar 5, 2020 at 11:05 AM Xiang Xiao wrote: > > > > On Thu, Mar 5, 2020 at 5:10 PM Abdelatif Guettouche > > wrote: > > > > > > Hi, > > > > > > We do not squash commits unless they are related (Which should have been > > > done by the OP in the firsr place). > > > > But even the patch related each other, it's better to keep the > > individual small patch instead merging into one big patch. Because if > > the contributor take the time to split the change into serveral small > > patch, which mean it's valuable to do so. > > > > it is important to keep the patch as it without modification, if the > > PR doesn't looks good, we can: > > 1.Suggest the contributor modify the change and update PR again > > 2.Or merge the PR and create a new PR fix the remaining issue. > > Of course, the second approach should be selected only for the minor > > issue(coding style, naming etc). Each individual patch should pass the > > build and no regression first. > > > > > We actually encourage putting unrelated changes in separate commits. > > > > > > > Most people send PR contain the multiple commit just because all of > > them is related each other. If not, maintainer can request the > > contributor split PR into small ones. > > > > > > > > > > > On Thu, Mar 5, 2020, 05:26 Takashi Yamamoto > > > > > > wrote: > > > > > > > hi, > > > > > > > > it seems that in nuttx it's common to squash commits when merging pull > > > > requests. > > > > > > > > i'd like to suggest _not_ to do that because: > > > > * it makes cherry-pick/backport/revert cumbersome > > > > * larger changes are more difficult to read/understand > > > > > > > > on the other hand, i can think of only little benefit. > > > > * aesthetic reasons? > > > > * marginally save the repo size growth? > > > > > > > > how do you think? > > > >
RE: squashing commits or not
Agree! If you read the work flow I suggested you will see it is a rebase until review WF. Noise should always be squashed. -Original Message- From: Abdelatif Guettouche [mailto:abdelatif.guettou...@gmail.com] Sent: Thursday, March 05, 2020 4:53 AM To: dev@nuttx.apache.org Subject: Re: squashing commits or not > Because if > the contributor take the time to split the change into serveral small > patch, which mean it's valuable to do so. I agree, but this isn't always the case. It is okay to have PRs with multiple commits, as you mentioned, we can even request to do so. However, some of the commits are just a sequence of the same change. That's valuable for a WIP, but IMO should've been squashed before creating the PR. On Thu, Mar 5, 2020 at 11:05 AM Xiang Xiao wrote: > > On Thu, Mar 5, 2020 at 5:10 PM Abdelatif Guettouche > wrote: > > > > Hi, > > > > We do not squash commits unless they are related (Which should have been > > done by the OP in the firsr place). > > But even the patch related each other, it's better to keep the > individual small patch instead merging into one big patch. Because if > the contributor take the time to split the change into serveral small > patch, which mean it's valuable to do so. > > it is important to keep the patch as it without modification, if the > PR doesn't looks good, we can: > 1.Suggest the contributor modify the change and update PR again > 2.Or merge the PR and create a new PR fix the remaining issue. > Of course, the second approach should be selected only for the minor > issue(coding style, naming etc). Each individual patch should pass the > build and no regression first. > > > We actually encourage putting unrelated changes in separate commits. > > > > Most people send PR contain the multiple commit just because all of > them is related each other. If not, maintainer can request the > contributor split PR into small ones. > > > > > > > On Thu, Mar 5, 2020, 05:26 Takashi Yamamoto > > > > wrote: > > > > > hi, > > > > > > it seems that in nuttx it's common to squash commits when merging pull > > > requests. > > > > > > i'd like to suggest _not_ to do that because: > > > * it makes cherry-pick/backport/revert cumbersome > > > * larger changes are more difficult to read/understand > > > > > > on the other hand, i can think of only little benefit. > > > * aesthetic reasons? > > > * marginally save the repo size growth? > > > > > > how do you think? > > >
Re: squashing commits or not
> Because if > the contributor take the time to split the change into serveral small > patch, which mean it's valuable to do so. I agree, but this isn't always the case. It is okay to have PRs with multiple commits, as you mentioned, we can even request to do so. However, some of the commits are just a sequence of the same change. That's valuable for a WIP, but IMO should've been squashed before creating the PR. On Thu, Mar 5, 2020 at 11:05 AM Xiang Xiao wrote: > > On Thu, Mar 5, 2020 at 5:10 PM Abdelatif Guettouche > wrote: > > > > Hi, > > > > We do not squash commits unless they are related (Which should have been > > done by the OP in the firsr place). > > But even the patch related each other, it's better to keep the > individual small patch instead merging into one big patch. Because if > the contributor take the time to split the change into serveral small > patch, which mean it's valuable to do so. > > it is important to keep the patch as it without modification, if the > PR doesn't looks good, we can: > 1.Suggest the contributor modify the change and update PR again > 2.Or merge the PR and create a new PR fix the remaining issue. > Of course, the second approach should be selected only for the minor > issue(coding style, naming etc). Each individual patch should pass the > build and no regression first. > > > We actually encourage putting unrelated changes in separate commits. > > > > Most people send PR contain the multiple commit just because all of > them is related each other. If not, maintainer can request the > contributor split PR into small ones. > > > > > > > On Thu, Mar 5, 2020, 05:26 Takashi Yamamoto > > wrote: > > > > > hi, > > > > > > it seems that in nuttx it's common to squash commits when merging pull > > > requests. > > > > > > i'd like to suggest _not_ to do that because: > > > * it makes cherry-pick/backport/revert cumbersome > > > * larger changes are more difficult to read/understand > > > > > > on the other hand, i can think of only little benefit. > > > * aesthetic reasons? > > > * marginally save the repo size growth? > > > > > > how do you think? > > >
Re: squashing commits or not
On Thu, Mar 5, 2020 at 5:10 PM Abdelatif Guettouche wrote: > > Hi, > > We do not squash commits unless they are related (Which should have been > done by the OP in the firsr place). But even the patch related each other, it's better to keep the individual small patch instead merging into one big patch. Because if the contributor take the time to split the change into serveral small patch, which mean it's valuable to do so. it is important to keep the patch as it without modification, if the PR doesn't looks good, we can: 1.Suggest the contributor modify the change and update PR again 2.Or merge the PR and create a new PR fix the remaining issue. Of course, the second approach should be selected only for the minor issue(coding style, naming etc). Each individual patch should pass the build and no regression first. > We actually encourage putting unrelated changes in separate commits. > Most people send PR contain the multiple commit just because all of them is related each other. If not, maintainer can request the contributor split PR into small ones. > > > On Thu, Mar 5, 2020, 05:26 Takashi Yamamoto > wrote: > > > hi, > > > > it seems that in nuttx it's common to squash commits when merging pull > > requests. > > > > i'd like to suggest _not_ to do that because: > > * it makes cherry-pick/backport/revert cumbersome > > * larger changes are more difficult to read/understand > > > > on the other hand, i can think of only little benefit. > > * aesthetic reasons? > > * marginally save the repo size growth? > > > > how do you think? > >
Re: squashing commits or not
Hi, We do not squash commits unless they are related (Which should have been done by the OP in the firsr place). We actually encourage putting unrelated changes in separate commits. On Thu, Mar 5, 2020, 05:26 Takashi Yamamoto wrote: > hi, > > it seems that in nuttx it's common to squash commits when merging pull > requests. > > i'd like to suggest _not_ to do that because: > * it makes cherry-pick/backport/revert cumbersome > * larger changes are more difficult to read/understand > > on the other hand, i can think of only little benefit. > * aesthetic reasons? > * marginally save the repo size growth? > > how do you think? >
