As you can see, I have been tried to forward relevant emails from this
thread. There are at least two and maybe three that I cannot find.
First there is the text which I appended to Nathan's workflow:
Proposed Work Flow
Proposed Steps from Contribution to Commit
I think the work flow should be like this:
1.
PR or patch received (where? how? Via incubator email?
2.
Triggers automated checking:
1.
Verify that it follows the coding standard, and
2.
verify that the build is not broken.
If either fail, ask the contributor to fix the problem and resubmit
the change.
3.
PMC should triage and assign the change to a committer. PMC may
also review for conformance with the Inviolables If this review
fails, the change is declined.
4.
Committer performs final review for technical correctness and
conformance to the Inviolables. If this review fails, the change is
declined otherwise the committer commits the change.
5.
2:42 PM <https://nuttx.slack.com/archives/GM2JH0P3M/p1575146565176000>
Step 1
Changes should include some information about how to test the change.
For modifications to code that is tested by existing configurations, we
would need to know the relevant configurations settings. From that we
should be able to select a set of relevant test configurations.
If the change is a new feature, then it may not be testable using any
existing configuration. In that case, we will have to insist that the
change be accompanied by a configuration that can be used for testing.
Step 2
For now we just need this minimum, but this should extend in the future
as we aim for a higher level of quality assurance.
Step 2a)The NuttX style verification tool, nxstyle, should be used to
check coding style. If the submission does not follow the NuttX coding
style, we will need to ask the contributor to update the change so that
it does.
Nxstyle is an imperfect tool, however. We probably need to manually
check any failed output to verify that the failures are not false alarms.
Step 2b) The brute force Jenkins-style testing is not useful here.
Rather, we need a smarter build. We need to build configurations that
ACTUALLY build the code that is changed by the contribution. Per step
1) the contributor has either provided a new test configuration (which
should be included) as a part of the change, or 2) the contributor has
provided the relevant configuration settings for testing the change.
In the latter case, we should be able to build a test configuration list
by selecting existing configuration that include these configuration
settings.
Step 2c) This is where we may want to add hardware-in-the-loop testing
with a reference board in the future.
Step 3
There should be a list of all committers with the areas in which they
have the best expertise. Assigning a change to a committer should be
simply picking the person with the best expertise, but also accounting
for any backlog. Other committers may need to take up the slack.
Step 4
Ultimately, it is the committer who is responsible for assuring that (1)
the change is technically correct, complete, and of the highest
quality. And that (2) the change is consistent with all of the
principles of the Inviolables: The change must not violate the portable
POSIX interface, the change must conform to the architectural principles
of the OS, the change must not expose any platform dependencies that
would have any impact on other users of NuttX.
At this point, the committer should be confident that the change is in
full compliance with the coding standard and will not break the build.
Then there is Justin Mclean's response to this. I cannot find it, but
it is where the phrase "Scratch what itches" comes from. In Step 4,
Justin recommend not being so former. In most Apache projects,
commiters just "scratch what itches" meaning that they review and merge
commits that are interesting to them.
There is possibly a third email that I cannot find from David Sidrane
that had some thoughts about the work flow. I can't find it and I don't
recall the content. If someone else knows the email that I am referring
to, it would be good to have on this thread as well.
Greg
