search

opencomputeproject / onie

Open Network Install Environment
https://opencomputeproject.github.io/onie
Other
605 stars 376 forks source link

Merge/pull requests should be accepted to take advantage of GitHub features #952

Open paulmenzel opened 3 years ago

paulmenzel commented 3 years ago

Factored out from merge/pull request https://github.com/opencomputeproject/onie/pull/949:

Out of curiosity, why aren’t you just accepting the merge/pull request but manually cherry-pick the commit?

Hi - it's a workflow consistency thing. Sometimes submitted commits need to be squashed to present as one functional change, or I'll do some cleanup on the comments for clarity, and in those cases I can just handle it without having to ask for a resubmission, and it'll have me down as having signed off on it. If it doesn't need any of that, then the same workflow applies -it just gets rubber stamped and committed.

paulmenzel commented 3 years ago

Thank you for sharing your motivation. In my opinion, it’s disrupting the GitHub workflow, people are used to.

  1. How can accepted and rejected merge/pull requests be distinguished? Currently GitHub shows:

    Closed with unmerged commits

    This pull request is closed, but the paulmenzel:remove-trailing-whitespace branch has unmerged commits. You can delete this branch if you wish.

    So the metadata is “incorrect”, and users have to read the comments.

  2. The git history/log does not document, what merge/pull request it was, making it hard to easily go and find a possible discussion in the merge/pull request.

  3. In my opinion, you do not need to sign off on commits, because you pressing the Merge button and you getting documented as this person means the same thing from my point of view.

  4. Also – now having a little insight into the quality of some of the submissions to ONIE – I understand, you need to amend certain things, and doing it yourself instead of “teaching” others saves time. You should just force push your changes to the branch, cf. [Allow edits from maintainers]]1, which is automatically set (for ONIE(?)). At least in my merge/pull request https://github.com/opencomputeproject/onie/pull/950 the box is checked by default.

  5. ONIE has become the standard and manufacturers have to contribute upstream. That puts ONIE in a good position, being able to raise the quality standard for contributions saving you and the users time in the end. Referring to the https://github.com/opencomputeproject/onie/blob/master/CONTRIBUTING.md should be enough, and – when having questions – referring contributors to their colleagues, where certainly some are knowledgeable of FLOSS contributions.