[
https://issues.apache.org/jira/browse/BEAM-6122?focusedWorklogId=169405&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-169405
]
ASF GitHub Bot logged work on BEAM-6122:
----------------------------------------
Author: ASF GitHub Bot
Created on: 26/Nov/18 17:37
Start Date: 26/Nov/18 17:37
Worklog Time Spent: 10m
Work Description: apilloud commented on a change in pull request #7129:
[BEAM-6122] Update committer guidelines
URL: https://github.com/apache/beam/pull/7129#discussion_r236355354
##########
File path: website/src/contribute/committer-guide.md
##########
@@ -77,15 +95,25 @@ At some point in the review process, the change to the
codebase will be
complete. However, the pull request may have a collection of review-related
commits that are not meaningful to preserve in the history. The reviewer should
give the LGTM and then request that the author of the pull request rebase,
-squash, split, etc, the commits, so that the history is most useful. Favor
-commits that do just one thing. The commit is the smallest unit of easy
+squash, split, etc, the commits, so that the history is most useful:
+* Favor commits that do just one thing. The commit is the smallest unit of easy
rollback; it is easy to roll back many commits, or a whole pull request, but
harder to roll back part of a commit.
+* Commit messages should tag JIRAs and be otherwise descriptive.
+It should later not be necessary to find a merge or first PR commit to find
out what caused a change.
+* Squash the "Fixup!", "Address comments" type of commits that resulted from
review iterations.
+
+While it is preferred that authors squash commits after review is complete,
+there may be situations where is more practical for the committer to handle
this
+(such as when the action to be taken is obvious or the author isn't available).
+The committer may use the "Squash and merge" option in Github (or modify the
PR commits in other ways).
+The committer is ultimately responsible and we "trust the committer's
judgment"!
## Merging it!
After all the tests pass, there should be a green merge button at the bottom of
-the pull request. There are multiple choices and you should choose "Merge pull
+the pull request. There are multiple choices. Unless you want to squash commits
+as part of the merge (see above) you should choose "Merge pull
request" (the default). This preserves the commit history and adds a merge
Review comment:
`Merge pull request` actually defaults to whatever operation you did last
time. Can you remove `(the default)` and replace it with `and ensure "Create a
merge commit" is selected from the drop down.`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 169405)
Time Spent: 40m (was: 0.5h)
> Update committer guidelines
> ---------------------------
>
> Key: BEAM-6122
> URL: https://issues.apache.org/jira/browse/BEAM-6122
> Project: Beam
> Issue Type: Task
> Components: website
> Reporter: Thomas Weise
> Assignee: Thomas Weise
> Priority: Major
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Per discussion in
> [https://lists.apache.org/thread.html/6d922820d6fc352479f88e5c8737f2c8893ddb706a1e578b50d28948@%3Cdev.beam.apache.org%3E]
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
