You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Xiang Xiao <xi...@gmail.com> on 2020/01/01 05:25:07 UTC
Re: End-of-year Clean-up
On Wed, Jan 1, 2020 at 5:09 AM Gregory Nutt <sp...@gmail.com> wrote:
>
>
> > Would it make sense, then, to begin a transition period now? That is,
> > start a gradual move from the current state where Greg is reviewing
> > and merging all changes, toward the direction where other committers
> > are reviewing/merging changes. Perhaps, for the next couple of weeks,
> > Greg could continue reviewing and merging to the more finicky areas
> > like the build system, while other committers start getting their feet
> > wet by reviewing and merging changes to areas less likely to bring
> > breakage? I think Greg should be mentoring us in the art of handling
> > contributions, instead of doing all the drudge work himself.
>
> That sounds like a good idea. Let me summarize what I do. This is /not
> /the official workflow! It is basically the legacy way I have been
> doing things translated in to this new environment. It really only
> specifically only addresses PRs, but patches would be similar: You
> would commit the patch to master or a development branch instead of
> merging the PR. Otherwise it is the same:
>
> * First review the change. If it looks risky, ask for other,
> appropriate people to review the change too.
> * If the change is a trivial change (like a typo fix) or an obvious
> change (like correcting a bad definition), I just merge it directly
> to master and we are done.
> * Otherwise, review the change. Wait for other review comments if you
> have requested them. You may need to ask the contributor to fix
> certain things and force push an update.
> * When you are satisfied, merge the change onto the 'dev' branch (or a
> custom branch of your choosing).
>
> * First make sure that the dev branch is up-to-date
> * cd incubator-nuttx
> * git checkout dev
> * git rebase master
> * git push [--force] origin dev
> * --force is a little dangerous. Don't use it if other people are
> using the dev branch. This is a good argument for merging the
> change onto a custom branch.
> * Set the base branch of the PR to the dev branch per
>
https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-base-branch-of-a-pull-request
> * Squash merge the change onto the dev branch.
>
> * Capture the .c and .h files that were added or modified on the 'dev'
> branch. Run nxstyle on every such file. Modify the code as
> necessary so that all .c and .h files pass the test (using good
> judgement if you don't agree with nxstyle).
> * Squash merge the 'dev' branch onto master. The change on master
> will include both the original PR and your coding style fixes.
> Again, this assumes you are the only user of dev and another reason
> to consider using a custom temporary branch.
> * Finally, refresh the dev branch (as above) or delete your custom,
> temporary branch.
>
But I have some concern about this process(especially for style and squash).
For example, https://github.com/apache/incubator-nuttx/pull/17 contain 5
patches(change 52 files and 915 lines) to clean up the network stack, but
the final result on master have one huge patch.
The problem include:
1.It's difficult to find all modification relate to a particular issue
2.It's difficult to revert one patch if that patch make the regression
3.The owner info will lost if the patchset is contrubted by multiple authors
4.The owner mayn't agree the change made by committer without any discussion
5.It's hard to master the real change if style patch squash into the
functionality change
And the most important is that the whole complex process is done manually
and directly merge into master, NO ANY TOOL has the chance to verify the
change.
Why we can't "Rebase and merge" PR without any modification after reviewing
to keep the full history?
[image: Annotation 2020-01-01 124839.jpg]
If there are any issues, the best way is:
1.Add the comment in PR
2.Owner fix the problem or explain the reason
3.Update the PR again
And github can trigger our tool to check PR when the owner create or update
PR.
> Notice that there is no attempt to prove that the code builds correctly
> and, hence, incorporating the change could easily cause breakage. That
> is, in my opinion, more than you can expect from a purely manual
> process. I used to run extensive build testing of over 408 ARM
> configurations which detects build problems in many (but not all
> cases). I have stopped doing that, however.
We need fix the problem: once PR is created, the build job should start
automatically to very the change doesn't break the build.
Actually, the committer don't need review the code before PR pass all
automation check.
>
> So who wants to help? It is really kind of entertaining, provided that
> you aren't swamped with more than you can do and provided it does not
> take over your life (as it did mine).
>
> > .... Changes are only merged after an appropriate level of review.
>
> If someone puts a person down as a review of a PR, then we all need to
> take responsibility to provide the review, comments, and approval. For
> example, the nxstyle change that was haggled for several days had
> reviews requested from 6 people (as I recall). I think I am the only
> person that both reviewed, commented, and approved the change.
>
> Greg
>
>
Re: End-of-year Clean-up
Posted by Gregory Nutt <sp...@gmail.com>.
> I want to help reviewing the patches and be guided by you to do it well.
Thanks, Alan!
Imagine if we all tried to dispose on one PR per day. That would be
eleven PRs per day, 77 PRs per week, week could keep up with things
without stressing anyone.
Of course, it will be even easier when the fully agreed to, automated
work flow is in place. But we need to handle changes however we can
until that day arrives. I think we could have work flow requirements in
a 1-2 weeks and possibly some automation help a few weeks after that.
Greg
Re: End-of-year Clean-up
Posted by Alan Carvalho de Assis <ac...@gmail.com>.
Hi Nathan
On Thursday, January 2, 2020, Nathan Hartman <ha...@gmail.com>
wrote:
> On Wed, Jan 1, 2020 at 7:27 PM Alan Carvalho de Assis <ac...@gmail.com>
> wrote:
>
>> HI Nathan,
>>
>> On 1/1/20, Nathan Hartman <ha...@gmail.com> wrote:
>> > On Wed, Jan 1, 2020 at 1:14 PM Alan Carvalho de Assis <
acassis@gmail.com
>> >
>> > wrote:
>> >> On 1/1/20, Gregory Nutt <sp...@gmail.com> wrote:
>> >> > * Brennan has done the Confluence pages, investigated Jira, create
>> >> > the
>> >> > initial workflow page
>> >> > * Nathan have been staying busy with the workflow
>> >> > * Abdelatif has been working a lot with PPMC/committer membership,
>> >> > monthly status report
>> >> > * I have been doing all of the commits.
>> >> >
>> >> > What about the remaining seven people? Is there no one who wants to
>> >> > get
>> >> > their feet wet? If we are going survive the next months, we all
have
>> >> > to
>> >> > pitch in and work together.
>> >> >
>> >>
>> >> I want to help reviewing the patches and be guided by you to do it
well.
>> >>
>> >> In this mean time I took a look at the Workflow page
>> >> (
>>
https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton
>> )
>> >> and I think it is not describing the Process, so I will try to get
>> >> your suggested steps and include it there.
>> >>
>> >> We could use the Mynewt page as reference:
>> >>
>> >>
>>
https://cwiki.apache.org/confluence/display/MYNEWT/Submitting+Pull+Requests
>> >>
>> >> Not their process, but their structure more direct to the goal.
>> >
>> > Thank you Alan.
>> >
>> > I will greatly appreciate help in getting the workflow complete.
>> >
>> > Feel free to edit and reorganize anything in that document to help
>> > make it more readable and helpful.
>> >
>> > Please keep in mind that our document addresses more things that
>> > aren't discussed in MYNEWT's workflow:
>> >
>>
>> Yes, it is true.
>>
>> After comparing things I think the "NuttX Workflow" will be to much
>> for a newcomer read if he/she just want to submit a patch/PR.
>>
>> I think we could have the Workflow document (mostly focused on
>> Committer) and a document simpler to explain how to submit patches/PR.
>> We could recommend reading but Workflow, but it could be optional.
>
>
> Perhaps there will be numerous subjects (for example we didn't discuss
> release process yet) and especially after we fill in a lot of details, it
> will be even longer. So maybe there should be a landing page with links to
> separate pages for each subject.
>
>
Exactly!
Although it is good that people know how to project works internally, it is
not necessary for someone that just want to submit a patch/PR.
BR,
Alan
Re: End-of-year Clean-up
Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Jan 1, 2020 at 7:27 PM Alan Carvalho de Assis <ac...@gmail.com>
wrote:
> HI Nathan,
>
> On 1/1/20, Nathan Hartman <ha...@gmail.com> wrote:
> > On Wed, Jan 1, 2020 at 1:14 PM Alan Carvalho de Assis <acassis@gmail.com
> >
> > wrote:
> >> On 1/1/20, Gregory Nutt <sp...@gmail.com> wrote:
> >> > * Brennan has done the Confluence pages, investigated Jira, create
> >> > the
> >> > initial workflow page
> >> > * Nathan have been staying busy with the workflow
> >> > * Abdelatif has been working a lot with PPMC/committer membership,
> >> > monthly status report
> >> > * I have been doing all of the commits.
> >> >
> >> > What about the remaining seven people? Is there no one who wants to
> >> > get
> >> > their feet wet? If we are going survive the next months, we all have
> >> > to
> >> > pitch in and work together.
> >> >
> >>
> >> I want to help reviewing the patches and be guided by you to do it well.
> >>
> >> In this mean time I took a look at the Workflow page
> >> (
> https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton
> )
> >> and I think it is not describing the Process, so I will try to get
> >> your suggested steps and include it there.
> >>
> >> We could use the Mynewt page as reference:
> >>
> >>
> https://cwiki.apache.org/confluence/display/MYNEWT/Submitting+Pull+Requests
> >>
> >> Not their process, but their structure more direct to the goal.
> >
> > Thank you Alan.
> >
> > I will greatly appreciate help in getting the workflow complete.
> >
> > Feel free to edit and reorganize anything in that document to help
> > make it more readable and helpful.
> >
> > Please keep in mind that our document addresses more things that
> > aren't discussed in MYNEWT's workflow:
> >
>
> Yes, it is true.
>
> After comparing things I think the "NuttX Workflow" will be to much
> for a newcomer read if he/she just want to submit a patch/PR.
>
> I think we could have the Workflow document (mostly focused on
> Committer) and a document simpler to explain how to submit patches/PR.
> We could recommend reading but Workflow, but it could be optional.
Perhaps there will be numerous subjects (for example we didn't discuss
release process yet) and especially after we fill in a lot of details, it
will be even longer. So maybe there should be a landing page with links to
separate pages for each subject.
Nathan
Re: End-of-year Clean-up
Posted by Alan Carvalho de Assis <ac...@gmail.com>.
HI Nathan,
On 1/1/20, Nathan Hartman <ha...@gmail.com> wrote:
> On Wed, Jan 1, 2020 at 1:14 PM Alan Carvalho de Assis <ac...@gmail.com>
> wrote:
>> On 1/1/20, Gregory Nutt <sp...@gmail.com> wrote:
>> > * Brennan has done the Confluence pages, investigated Jira, create
>> > the
>> > initial workflow page
>> > * Nathan have been staying busy with the workflow
>> > * Abdelatif has been working a lot with PPMC/committer membership,
>> > monthly status report
>> > * I have been doing all of the commits.
>> >
>> > What about the remaining seven people? Is there no one who wants to
>> > get
>> > their feet wet? If we are going survive the next months, we all have
>> > to
>> > pitch in and work together.
>> >
>>
>> I want to help reviewing the patches and be guided by you to do it well.
>>
>> In this mean time I took a look at the Workflow page
>> (https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton)
>> and I think it is not describing the Process, so I will try to get
>> your suggested steps and include it there.
>>
>> We could use the Mynewt page as reference:
>>
>> https://cwiki.apache.org/confluence/display/MYNEWT/Submitting+Pull+Requests
>>
>> Not their process, but their structure more direct to the goal.
>
> Thank you Alan.
>
> I will greatly appreciate help in getting the workflow complete.
>
> Feel free to edit and reorganize anything in that document to help
> make it more readable and helpful.
>
> Please keep in mind that our document addresses more things that
> aren't discussed in MYNEWT's workflow:
>
Yes, it is true.
After comparing things I think the "NuttX Workflow" will be to much
for a newcomer read if he/she just want to submit a patch/PR.
I think we could have the Workflow document (mostly focused on
Committer) and a document simpler to explain how to submit patches/PR.
We could recommend reading but Workflow, but it could be optional.
> * Where is the code and how to get a copy of it
>
> * The workflow itself (currently lacks all details)
>
> * Criteria for acceptance, which currently is very basic and has no
> details but it will gradually develop over time
>
> * A reference for us committers, to guide us in merging the
> contributions safely to git without messing up the repositories
>
> Any area(s) where you can help, we'll all appreciate it very much.
>
Ok, I will keep getting feedbacks and editing it.
Thank you very much!
BR,
Alan
Re: End-of-year Clean-up
Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Jan 1, 2020 at 1:14 PM Alan Carvalho de Assis <ac...@gmail.com> wrote:
> On 1/1/20, Gregory Nutt <sp...@gmail.com> wrote:
> > * Brennan has done the Confluence pages, investigated Jira, create the
> > initial workflow page
> > * Nathan have been staying busy with the workflow
> > * Abdelatif has been working a lot with PPMC/committer membership,
> > monthly status report
> > * I have been doing all of the commits.
> >
> > What about the remaining seven people? Is there no one who wants to get
> > their feet wet? If we are going survive the next months, we all have to
> > pitch in and work together.
> >
>
> I want to help reviewing the patches and be guided by you to do it well.
>
> In this mean time I took a look at the Workflow page
> (https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton)
> and I think it is not describing the Process, so I will try to get
> your suggested steps and include it there.
>
> We could use the Mynewt page as reference:
>
> https://cwiki.apache.org/confluence/display/MYNEWT/Submitting+Pull+Requests
>
> Not their process, but their structure more direct to the goal.
Thank you Alan.
I will greatly appreciate help in getting the workflow complete.
Feel free to edit and reorganize anything in that document to help
make it more readable and helpful.
Please keep in mind that our document addresses more things that
aren't discussed in MYNEWT's workflow:
* Where is the code and how to get a copy of it
* The workflow itself (currently lacks all details)
* Criteria for acceptance, which currently is very basic and has no
details but it will gradually develop over time
* A reference for us committers, to guide us in merging the
contributions safely to git without messing up the repositories
Any area(s) where you can help, we'll all appreciate it very much.
Thanks again,
Nathan
Re: End-of-year Clean-up
Posted by Alan Carvalho de Assis <ac...@gmail.com>.
Hi Greg,
On 1/1/20, Gregory Nutt <sp...@gmail.com> wrote:
> We need to reset the conversation because it has gotten so far off track
> that the point has been completely lost. We need to pay attention and
> not get distracted by the shiny objects.
>
> Repeating:
>
> > > Would it make sense, then, to begin a transition period now? That is,
> > > start a gradual move from the current state where Greg is reviewing
> > > and merging all changes, toward the direction */where other
> committers/*
> > > */are reviewing/merging changes/*. Perhaps, for the next couple of
> weeks,
> > > Greg could continue reviewing and merging to the more finicky areas
> > > like the build system, while /*other committers start getting their
> feet*/
> > > */wet by reviewing and merging changes/* to areas less likely to bring
> > > breakage? I think Greg should be mentoring us in the art of handling
> > > contributions, instead of doing all the drudge work himself.
> >
> > That sounds like a good idea. Let me summarize what I do. This is
> /*not*/
> > /*the official workflow*/! It is basically the/*legacy way*/ I have
> been
> > doing things translated in to this new environment. ...
>
> To that I got two responses critical the seat-of-the-pants workflow I am
> using and on critical of me personally. Not helpful.
>
> We need to revisit. There are /eleven /people who have committed to
> helping out as members of the PPMC or as commiters.
>
> * Brennan has done the Confluence pages, investigated Jira, create the
> initial workflow page
> * Nathan have been staying busy with the workflow
> * Abdelatif has been working a lot with PPMC/committer membership,
> monthly status report
> * I have been doing all of the commits.
>
> What about the remaining seven people? Is there no one who wants to get
> their feet wet? If we are going survive the next months, we all have to
> pitch in and work together.
>
I want to help reviewing the patches and be guided by you to do it well.
In this mean time I took a look at the Workflow page
(https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton)
and I think it is not describing the Process, so I will try to get
your suggested steps and include it there.
We could use the Mynewt page as reference:
https://cwiki.apache.org/confluence/display/MYNEWT/Submitting+Pull+Requests
Not their process, but their structure more direct to the goal.
BR,
Alan
Re: End-of-year Clean-up
Posted by Gregory Nutt <sp...@gmail.com>.
We need to reset the conversation because it has gotten so far off track
that the point has been completely lost. We need to pay attention and
not get distracted by the shiny objects.
Repeating:
> > Would it make sense, then, to begin a transition period now? That is,
> > start a gradual move from the current state where Greg is reviewing
> > and merging all changes, toward the direction */where other
committers/*
> > */are reviewing/merging changes/*. Perhaps, for the next couple of
weeks,
> > Greg could continue reviewing and merging to the more finicky areas
> > like the build system, while /*other committers start getting their
feet*/
> > */wet by reviewing and merging changes/* to areas less likely to bring
> > breakage? I think Greg should be mentoring us in the art of handling
> > contributions, instead of doing all the drudge work himself.
>
> That sounds like a good idea. Let me summarize what I do. This is
/*not*/
> /*the official workflow*/! It is basically the/*legacy way*/ I have been
> doing things translated in to this new environment. ...
To that I got two responses critical the seat-of-the-pants workflow I am
using and on critical of me personally. Not helpful.
We need to revisit. There are /eleven /people who have committed to
helping out as members of the PPMC or as commiters.
* Brennan has done the Confluence pages, investigated Jira, create the
initial workflow page
* Nathan have been staying busy with the workflow
* Abdelatif has been working a lot with PPMC/committer membership,
monthly status report
* I have been doing all of the commits.
What about the remaining seven people? Is there no one who wants to get
their feet wet? If we are going survive the next months, we all have to
pitch in and work together.
Greg
Re: End-of-year Clean-up
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> I don't appreciate a lot of advice and criticism from people who contribute nothing to this process.
>
I would try to assume good intent from the people who responded here. You want people to contribute and grow the community,
Thanks,
Justin
Re: End-of-year Clean-up
Posted by Gregory Nutt <sp...@gmail.com>.
> Sorry, I misunderstand that you suggest this will be the new workflow.
Okay. No problem. Although I did feel like I was being attacked from
all sides for just trying to make to progress.
All discussion of the new workflow is happening here:
https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton
I invite everyone to participate.
Greg
Re: End-of-year Clean-up
Posted by Xiang Xiao <xi...@gmail.com>.
Sorry, I misunderstand that you suggest this will be the new workflow.
I agree that before the new process is ready, your process should continue
as before since you have most experience and insight how the whole thing
work together. But the committer is growing and most of us don't have that
capablity now, so the more strict workflow with automation tool can avoid
the risk.
On Wed, Jan 1, 2020 at 1:36 PM Gregory Nutt <sp...@gmail.com> wrote:
> That list just documents how I have handled the me-only workflow in the
> past. It is not the workflow that is being defined by Nathan and Brennan.
>
> I don't appreciate a lot of advice and criticism from people who
> contribute nothing to this process. If everyone thinks I can going
> continue handling all changes, with no help from any one but on criticism,
> you are all wrong. I won't do that. That is totally unfair.
>
> Thanks
> On 12/31/2019 11:25 PM, Xiang Xiao wrote:
>
>
>
> On Wed, Jan 1, 2020 at 5:09 AM Gregory Nutt <sp...@gmail.com> wrote:
> >
> >
> > > Would it make sense, then, to begin a transition period now? That is,
> > > start a gradual move from the current state where Greg is reviewing
> > > and merging all changes, toward the direction where other committers
> > > are reviewing/merging changes. Perhaps, for the next couple of weeks,
> > > Greg could continue reviewing and merging to the more finicky areas
> > > like the build system, while other committers start getting their feet
> > > wet by reviewing and merging changes to areas less likely to bring
> > > breakage? I think Greg should be mentoring us in the art of handling
> > > contributions, instead of doing all the drudge work himself.
> >
> > That sounds like a good idea. Let me summarize what I do. This is /not
> > /the official workflow! It is basically the legacy way I have been
> > doing things translated in to this new environment. It really only
> > specifically only addresses PRs, but patches would be similar: You
> > would commit the patch to master or a development branch instead of
> > merging the PR. Otherwise it is the same:
> >
> > * First review the change. If it looks risky, ask for other,
> > appropriate people to review the change too.
> > * If the change is a trivial change (like a typo fix) or an obvious
> > change (like correcting a bad definition), I just merge it directly
> > to master and we are done.
> > * Otherwise, review the change. Wait for other review comments if you
> > have requested them. You may need to ask the contributor to fix
> > certain things and force push an update.
> > * When you are satisfied, merge the change onto the 'dev' branch (or a
> > custom branch of your choosing).
> >
> > * First make sure that the dev branch is up-to-date
> > * cd incubator-nuttx
> > * git checkout dev
> > * git rebase master
> > * git push [--force] origin dev
> > * --force is a little dangerous. Don't use it if other people are
> > using the dev branch. This is a good argument for merging the
> > change onto a custom branch.
> > * Set the base branch of the PR to the dev branch per
> >
> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-base-branch-of-a-pull-request
> > * Squash merge the change onto the dev branch.
> >
> > * Capture the .c and .h files that were added or modified on the 'dev'
> > branch. Run nxstyle on every such file. Modify the code as
> > necessary so that all .c and .h files pass the test (using good
> > judgement if you don't agree with nxstyle).
> > * Squash merge the 'dev' branch onto master. The change on master
> > will include both the original PR and your coding style fixes.
> > Again, this assumes you are the only user of dev and another reason
> > to consider using a custom temporary branch.
> > * Finally, refresh the dev branch (as above) or delete your custom,
> > temporary branch.
> >
>
> But I have some concern about this process(especially for style and
> squash).
> For example, https://github.com/apache/incubator-nuttx/pull/17 contain 5
> patches(change 52 files and 915 lines) to clean up the network stack, but
> the final result on master have one huge patch.
> The problem include:
> 1.It's difficult to find all modification relate to a particular issue
> 2.It's difficult to revert one patch if that patch make the regression
> 3.The owner info will lost if the patchset is contrubted by multiple
> authors
> 4.The owner mayn't agree the change made by committer without any
> discussion
> 5.It's hard to master the real change if style patch squash into the
> functionality change
> And the most important is that the whole complex process is done manually
> and directly merge into master, NO ANY TOOL has the chance to verify the
> change.
> Why we can't "Rebase and merge" PR without any modification after
> reviewing to keep the full history?
> [image: Annotation 2020-01-01 124839.jpg]
> If there are any issues, the best way is:
> 1.Add the comment in PR
> 2.Owner fix the problem or explain the reason
> 3.Update the PR again
> And github can trigger our tool to check PR when the owner create or
> update PR.
>
>
> > Notice that there is no attempt to prove that the code builds correctly
> > and, hence, incorporating the change could easily cause breakage. That
> > is, in my opinion, more than you can expect from a purely manual
> > process. I used to run extensive build testing of over 408 ARM
> > configurations which detects build problems in many (but not all
> > cases). I have stopped doing that, however.
>
> We need fix the problem: once PR is created, the build job should start
> automatically to very the change doesn't break the build.
> Actually, the committer don't need review the code before PR pass all
> automation check.
>
> >
> > So who wants to help? It is really kind of entertaining, provided that
> > you aren't swamped with more than you can do and provided it does not
> > take over your life (as it did mine).
> >
> > > .... Changes are only merged after an appropriate level of review.
> >
> > If someone puts a person down as a review of a PR, then we all need to
> > take responsibility to provide the review, comments, and approval. For
> > example, the nxstyle change that was haggled for several days had
> > reviews requested from 6 people (as I recall). I think I am the only
> > person that both reviewed, commented, and approved the change.
> >
> > Greg
> >
> >
>
>
Re: End-of-year Clean-up
Posted by Gregory Nutt <sp...@gmail.com>.
That list just documents how I have handled the me-only workflow in the
past. It is not the workflow that is being defined by Nathan and Brennan.
I don't appreciate a lot of advice and criticism from people who
contribute nothing to this process. If everyone thinks I can going
continue handling all changes, with no help from any one but on
criticism, you are all wrong. I won't do that. That is totally unfair.
Thanks
On 12/31/2019 11:25 PM, Xiang Xiao wrote:
>
>
> On Wed, Jan 1, 2020 at 5:09 AM Gregory Nutt <spudaneco@gmail.com
> <ma...@gmail.com>> wrote:
> >
> >
> > > Would it make sense, then, to begin a transition period now? That is,
> > > start a gradual move from the current state where Greg is reviewing
> > > and merging all changes, toward the direction where other committers
> > > are reviewing/merging changes. Perhaps, for the next couple of weeks,
> > > Greg could continue reviewing and merging to the more finicky areas
> > > like the build system, while other committers start getting their feet
> > > wet by reviewing and merging changes to areas less likely to bring
> > > breakage? I think Greg should be mentoring us in the art of handling
> > > contributions, instead of doing all the drudge work himself.
> >
> > That sounds like a good idea. Let me summarize what I do. This is /not
> > /the official workflow! It is basically the legacy way I have been
> > doing things translated in to this new environment. It really only
> > specifically only addresses PRs, but patches would be similar: You
> > would commit the patch to master or a development branch instead of
> > merging the PR. Otherwise it is the same:
> >
> > * First review the change. If it looks risky, ask for other,
> > appropriate people to review the change too.
> > * If the change is a trivial change (like a typo fix) or an obvious
> > change (like correcting a bad definition), I just merge it directly
> > to master and we are done.
> > * Otherwise, review the change. Wait for other review comments if you
> > have requested them. You may need to ask the contributor to fix
> > certain things and force push an update.
> > * When you are satisfied, merge the change onto the 'dev' branch (or a
> > custom branch of your choosing).
> >
> > * First make sure that the dev branch is up-to-date
> > * cd incubator-nuttx
> > * git checkout dev
> > * git rebase master
> > * git push [--force] origin dev
> > * --force is a little dangerous. Don't use it if other people are
> > using the dev branch. This is a good argument for merging the
> > change onto a custom branch.
> > * Set the base branch of the PR to the dev branch per
> >
> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-base-branch-of-a-pull-request
> > * Squash merge the change onto the dev branch.
> >
> > * Capture the .c and .h files that were added or modified on the 'dev'
> > branch. Run nxstyle on every such file. Modify the code as
> > necessary so that all .c and .h files pass the test (using good
> > judgement if you don't agree with nxstyle).
> > * Squash merge the 'dev' branch onto master. The change on master
> > will include both the original PR and your coding style fixes.
> > Again, this assumes you are the only user of dev and another reason
> > to consider using a custom temporary branch.
> > * Finally, refresh the dev branch (as above) or delete your custom,
> > temporary branch.
> >
>
> But I have some concern about this process(especially for style and
> squash).
> For example, https://github.com/apache/incubator-nuttx/pull/17 contain
> 5 patches(change 52 files and 915 lines) to clean up the network
> stack, but the final result on master have one huge patch.
> The problem include:
> 1.It's difficult to find all modification relate to a particular issue
> 2.It's difficult to revert one patch if that patch make the regression
> 3.The owner info will lost if the patchset is contrubted by multiple
> authors
> 4.The owner mayn't agree the change made by committer without any
> discussion
> 5.It's hard to master the real change if style patch squash into the
> functionality change
> And the most important is that the whole complex process is done
> manually and directly merge into master, NO ANY TOOL has the chance to
> verify the change.
> Why we can't "Rebase and merge" PR without any modification after
> reviewing to keep the full history?
> Annotation 2020-01-01 124839.jpg
> If there are any issues, the best way is:
> 1.Add the comment in PR
> 2.Owner fix the problem or explain the reason
> 3.Update the PR again
> And github can trigger our tool to check PR when the owner create or
> update PR.
>
>
> > Notice that there is no attempt to prove that the code builds correctly
> > and, hence, incorporating the change could easily cause breakage. That
> > is, in my opinion, more than you can expect from a purely manual
> > process. I used to run extensive build testing of over 408 ARM
> > configurations which detects build problems in many (but not all
> > cases). I have stopped doing that, however.
>
> We need fix the problem: once PR is created, the build job should
> start automatically to very the change doesn't break the build.
> Actually, the committer don't need review the code before PR pass all
> automation check.
>
> >
> > So who wants to help? It is really kind of entertaining, provided that
> > you aren't swamped with more than you can do and provided it does not
> > take over your life (as it did mine).
> >
> > > .... Changes are only merged after an appropriate level of review.
> >
> > If someone puts a person down as a review of a PR, then we all need to
> > take responsibility to provide the review, comments, and approval. For
> > example, the nxstyle change that was haggled for several days had
> > reviews requested from 6 people (as I recall). I think I am the only
> > person that both reviewed, commented, and approved the change.
> >
> > Greg
> >
> >