Re: End-of-year Clean-up

[Alan Carvalho de Assis]

Hi Nathan

On Thursday, January 2, 2020, Nathan Hartman 
wrote:
> On Wed, Jan 1, 2020 at 7:27 PM Alan Carvalho de Assis 
> wrote:
>
>> HI Nathan,
>>
>> On 1/1/20, Nathan Hartman  wrote:
>> > On Wed, Jan 1, 2020 at 1:14 PM Alan Carvalho de Assis <
acas...@gmail.com
>> >
>> > wrote:
>> >> On 1/1/20, Gregory Nutt  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

[Nathan Hartman]

On Wed, Jan 1, 2020 at 7:27 PM Alan Carvalho de Assis 
wrote:

> HI Nathan,
>
> On 1/1/20, Nathan Hartman  wrote:
> > On Wed, Jan 1, 2020 at 1:14 PM Alan Carvalho de Assis  >
> > wrote:
> >> On 1/1/20, Gregory Nutt  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

[Alan Carvalho de Assis]

HI Nathan,

On 1/1/20, Nathan Hartman  wrote:
> On Wed, Jan 1, 2020 at 1:14 PM Alan Carvalho de Assis 
> wrote:
>> On 1/1/20, Gregory Nutt  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

[Nathan Hartman]

On Wed, Jan 1, 2020 at 1:14 PM Alan Carvalho de Assis  wrote:
> On 1/1/20, Gregory Nutt  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

[Gregory Nutt]

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

[Gregory Nutt]

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

[Justin Mclean]

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

[Xiang Xiao]

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  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  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 

Re: End-of-year Clean-up

[Gregory Nutt]

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 > 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 

Re: End-of-year Clean-up

[Xiang Xiao]

On Wed, Jan 1, 2020 at 5:09 AM Gregory Nutt  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 

Re: End-of-year Clean-up

[Nathan Hartman]

On Tue, Dec 31, 2019 at 4:50 PM Gregory Nutt  wrote:
> Actually, I think I recommended (through implication) that we should not
> use the 'dev' branch, but rather a custom, per-PR branch.  A single dev
> branch does not work for the reasons I mention above.  The worst is that
> many people are using the dev you cannot squash merge a change onto master.

I concur. It should *not* go to a big shared 'dev' branch. Individual
branch for that purpose is much better.

> And the changes would be moved to master with no testing.  No one is
> doing any manual testing and there is no purpose in maintaining
> potentially hundreds of changes on individual branches.  It does not
> serve either the project or the user.

Code won't be completely untested:

* Over time we will gain buildbots and whatnot, and these will perform
ongoing automated testing.

* Any interested community members can track master and put the latest
changes through their paces (I generally build my configurations each
day with the latest changes to make sure that nothing that is
important to me is broken).

* Part of the release process (which as not really been addressed yet)
will involve some kind of a "soak period" to allow contributors,
downstream, and any other interested parties to run tests (automated
and/or manual) and report issues.

Between all of these (and committers who are world-class developers) I
think the code will get pretty good review and testing.

Happy New Year!
Nathan

Re: End-of-year Clean-up

[Alan Carvalho de Assis]

On 12/31/19, Gregory Nutt  wrote:
>
>> The workflow will be improved in the future, but any interim workflow
>> will be the same flawed workflow that was used in the Bitbucket
>> repositories.. with some tweaks for working together better.
>>
> I think that the only objective is a short term one:  To keep our heads
> above water until the people and infrastructure catch up. That will
> require compromises in things that are not, in the long run, willing to
> compromise.
>

Completely agree! We have much work to keep the house organized!

Re: End-of-year Clean-up

[Alan Carvalho de Assis]

Hi Greg and Nathan,

On 12/31/19, Gregory Nutt  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.
>
> 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.
>
> 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.
>

Thank you for sharing this step-by-step process. I think it is fine,
we will always push the commits the dev branch and only when
everything is tested and stable it should be moved to the master.

In the future we could have some automated test to run nxstyle and
flag the PR as complaint before we review it. Projects using gerrit
automatically validates whether the patch could be applied and whether
it will break the build or not. Then the contributor needs to send a
fine patch before it be reviewed.

Happy new year! I wish we have a great Apache NuttX 2020 Year!

BR,

Alan

Re: End-of-year Clean-up

[Gregory Nutt]

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.

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.


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

[Nathan Hartman]

On Tue, Dec 31, 2019 at 3:01 PM Gregory Nutt  wrote:
> > That was actually necessary. We don't want to build a huge backlog.
>
> I still have some fears about what is going to happen after the
> holidays.  My experience is that things pick up slowly after the New
> Year.So we have another week or so until go back to their normal rates.
>
> Even then, I don't know what is going to happen.  Will people hold back
> changes because of the instability of the project?  Or will things
> continue as before?  I was disposing of around 50 changes per week for
> much of last year.  To get that done with a consistent level of quality,
> we will need clear workflow requirements and some automated help.
>
> I spent about a half a day every day dealing with changes.
>
> If things go back to a rate like that, we cannot allow a backlog to
> build up.. not even for one week.  So I suspect we will have to take
> some exceptional measures like this until we are fully ready.

The goal is to (eventually) remove this burden from Greg entirely and
have our (small but growing) army of committers reviewing and merging
changes.

I think it would benefit us as a community to start moving toward that
goal sooner rather than later. I know that the workflow document is
nowhere near ready. But I also think that the issues were discussed
and debated quite a lot and it seems that some unspoken consensus has
been reached on the direction and style of the workflow, even as many
details remain to be hashed out.

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.

Now this, of course, requires that everyone respect the idea that
being a committer doesn't give anyone license to bring in changes
willy-nilly. Changes are only merged after an appropriate level of
review. Until we have a workflow document that defines exactly what
that means, we will have to use our best judgment. If you're fixing
typos or bringing code into compliance with nxstyle without
introducing functional changes, that doesn't require much review at
all. If you're making functional changes, it's best to have some more
eyes on the code.

Thoughts?

Nathan

P.S., And meanwhile, we have to get that workflow document finished
(and will appreciate help from the community). It is at:
https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton

Re: End-of-year Clean-up

[Gregory Nutt]

That was actually necessary. We don't want to build a huge backlog.


I still have some fears about what is going to happen after the 
holidays.  My experience is that things pick up slowly after the New 
Year.    So we have another week or so until go back to their normal rates.


Even then, I don't know what is going to happen.  Will people hold back 
changes because of the instability of the project?  Or will things 
continue as before?  I was disposing of around 50 changes per week for 
much of last year.  To get that done with a consistent level of quality, 
we will need clear workflow requirements and some automated help.


I spent about a half a day every day dealing with changes.

If things go back to a rate like that, we cannot allow a backlog to 
build up.. not even for one week.  So I suspect we will have to take 
some exceptional measures like this until we are fully ready.


Greg

Re: End-of-year Clean-up

[Abdelatif Guettouche]

That was actually necessary. We don't want to build a huge backlog.


On Tue, Dec 31, 2019 at 1:32 PM Gregory Nutt  wrote:
>
> I propose that I merge all PRs under the "old workflow" so that we can
> start the New Year with a clean slate.  Let me know if anyone is opposed.
>
>

End-of-year Clean-up

[Gregory Nutt]

I propose that I merge all PRs under the "old workflow" so that we can 
start the New Year with a clean slate.  Let me know if anyone is opposed.