Helping someone is not a bad thing.
Consider the effect on the community and the code.
What use to happen with patches: patches were emailed, ***modified*** and
committed.
Submitted != Committed - Was that BAD Etiquette?
What use to happen with a PR: PR were submitted, 1-N more commits were made
AFTER merging the PR therefore modifying the code. This created risk, build
breakage and major commit spaghetti. Was that BAD Etiquette?
The **net** result was the correct thing was done from the code
perspective. - That was good, but not done right.
Well we could argue that while it was or was not bad etiquette, what it
really was, was bad workflow. That work should have been done on a branch,
reviewed, tested then merged.
This is what we are doing now.
The git hub tools allow us to collaborate, use a PR development effort to
educate, and get to a quality submission.
When with permission, a committer pushes to a contributor's PR the key
difference: The squashed commits of that PR are now atomic and not commit
spaghetti. Attribution is preserved. This is a WIN-WIN.
>From the community perspective we can help the contributor using things like
"Suggested Changes" and with giving them advice on how to proceed with
process (or an nxstyle flaw). We do not help them by criticizing them or
telling them things they can get from Nxstyle output.
For the push to PR option to be used, we have to give the contributor a
choice to:
a) Fix the code themselves
b) Offer to do it with or for them.
When you let them decide. Then it is not BAD Etiquette.
If it becomes a "Drive-by" PR and it has value to the project. Do a
continuation PR in the repo - attribute the work to them and fix it, test it
and commit it.
Things to consider:
We have to be patient: PR can run for a while.
We have to be respectful of people's time:
We need to provide working tooling and work flow.
Nxstyle needs to be fixed or replaced ASAP.
We cannot expect them to run bad the tools, and hold them
accountable the
Known issues.
A contribution is a solution that is shared by someone who may have done
the work for a day job. You cannot expect them to fix a coding style issues
immediately. The same goes for committers.
I will vote NO on limiting this feature. If it get rolled into the workflow
(Let’s call it: pork barreled) I will vote no on it as well.
David
-----Original Message-----
From: Gregory Nutt [mailto:spudan...@gmail.com]
Sent: Saturday, March 14, 2020 2:31 PM
To: dev@nuttx.apache.org
Subject: Question of PR Etiquette
Question: Github permits committer to modify the content of
contributors PRs through this mechanism:
https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork
I appreciate that this may be expeditious, but it feels like bad
etiquette to me. It seems like we should accept what the contributor
has offered, accept the changes, or allow them to modify the changes.
I can also appreciate that there may be cases where a committer requires
help to get things right, but that would be a special case.
Do we really want to encourage this as standard practice. My gut says
no, but I can also change my gut to match the consensus of the PPMC.
Whatever should be decided, the conclusion should be included in the
workflow document:
https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow.
If it is not permitted by the workflow, then it is forbidden, right?
I don't think we need to vote on this here. The proper text needs to be
included in the workflow document which we will, eventually approve by vote.
Greg
