Ceph Developers, There is a proper format for Merge Commits, which has been documented here: https://docs.ceph.com/docs/master/dev/developer_guide/basic-workflow/#proper... Kefu is quite keen for us to adhere to this format. If this needs to be beefed up or slimmed down, let me know. Zac Dover Upstream Docs Ceph
On Mon, Sep 7, 2020 at 8:25 AM John Zachary Dover <zac.dover@gmail.com> wrote:
Ceph Developers,
There is a proper format for Merge Commits, which has been documented here:
https://docs.ceph.com/docs/master/dev/developer_guide/basic-workflow/#proper...
Kefu is quite keen for us to adhere to this format.
If this needs to be beefed up or slimmed down, let me know.
Note: src/script/ptl-tool.py is a script tool for merging pull requests that automatically adds "Reviewed-by" to the merge commit message for you. It adds the message for GitHub review approvals or comments indicating "Reviewed-by...". -- Patrick Donnelly, Ph.D. He / Him / His Principal Software Engineer Red Hat Sunnyvale, CA GPG: 19F28A586F808C2402351B93C3301A3E258DD79D
On 9/8/20 1:06 AM, Patrick Donnelly wrote:
On Mon, Sep 7, 2020 at 8:25 AM John Zachary Dover <zac.dover@gmail.com> wrote:
There is a proper format for Merge Commits, which has been documented here:
https://docs.ceph.com/docs/master/dev/developer_guide/basic-workflow/#proper...
Kefu is quite keen for us to adhere to this format.
If this needs to be beefed up or slimmed down, let me know.
Thanks! This seems to be the format that github uses by default (even though one needs to add the "Reviewed-by:" lines manually). BTW, github also adds a comment about the pull request ID and the source branch, e.g.: [SNIP] commit b5e47e7bfaa4717cee5b808546260b9484c585ae (HEAD -> master, upstream/master) Merge: 7f2135648ed 44320a0d0c5 Author: Kefu Chai <kchai@redhat.com> Date: Fri Sep 4 17:14:10 2020 +0800 Merge pull request #36059 from liu-chunmei/seastore-extentmap-tree crimson/seastore: add extent map tree Reviewed-by: Samuel Just <sjust@redhat.com> [SNIP] I think it would be helpful to make at least the pull request ID a mandatory piece of information of a merge commt as well, this helps to quickly trace down the origin and related conversations about a contribution.
Note: src/script/ptl-tool.py is a script tool for merging pull requests that automatically adds "Reviewed-by" to the merge commit message for you. It adds the message for GitHub review approvals or comments indicating "Reviewed-by...".
Nice, good to know. But this requires push privileges into the corresponding branch, correct? Or is that a give if one has the permissions to merge PRs via the github web interface? I haven't dared trying pushing directly into master yet ;) By the way, ptl-tool.py seems to use a slightly different format than github when it comes to referring to the pull request: [SNIP] Author: Patrick Donnelly <pdonnell@redhat.com> Date: Thu Sep 3 10:08:42 2020 -0700 Merge PR #36546 into master * refs/pull/36546/head: vstart_runner: log commands in a more usable form Reviewed-by: Patrick Donnelly <pdonnell@redhat.com> Reviewed-by: Ernesto Puerta <epuertat@redhat.com> [SNIP] pt-tool indicates the destination branch, while github focuses on the source branch. Is there a point in trying to have a more consistent way of referring to the pull request ID and the branches involved? Lenz -- SUSE Software Solutions Germany GmbH - Maxfeldstr. 5 - 90409 Nuernberg GF: Felix Imendörffer, HRB 36809 (AG Nürnberg)
Hi Lenz, On Tue, Sep 8, 2020 at 12:02 AM Lenz Grimmer <lgrimmer@suse.com> wrote:
Note: src/script/ptl-tool.py is a script tool for merging pull requests that automatically adds "Reviewed-by" to the merge commit message for you. It adds the message for GitHub review approvals or comments indicating "Reviewed-by...".
Nice, good to know. But this requires push privileges into the corresponding branch, correct?
I believe so.
Or is that a give if one has the permissions to merge PRs via the github web interface? I haven't dared trying pushing directly into master yet ;)
By the way, ptl-tool.py seems to use a slightly different format than github when it comes to referring to the pull request:
[SNIP] Author: Patrick Donnelly <pdonnell@redhat.com> Date: Thu Sep 3 10:08:42 2020 -0700
Merge PR #36546 into master
* refs/pull/36546/head: vstart_runner: log commands in a more usable form
Reviewed-by: Patrick Donnelly <pdonnell@redhat.com> Reviewed-by: Ernesto Puerta <epuertat@redhat.com> [SNIP]
pt-tool indicates the destination branch, while github focuses on the source branch. Is there a point in trying to have a more consistent way of referring to the pull request ID and the branches involved?
It was a choice I made early on but not for any particular good reason. I don't really see a point in changing anything as the information about the source branch on Github is not particularly useful. -- Patrick Donnelly, Ph.D. He / Him / His Principal Software Engineer Red Hat Sunnyvale, CA GPG: 19F28A586F808C2402351B93C3301A3E258DD79D
pt-tool indicates the destination branch, while github focuses on the source branch. Is there a point in trying to have a more consistent way of referring to the pull request ID and the branches involved?
It was a choice I made early on but not for any particular good reason. I don't really see a point in changing anything as the information about the source branch on Github is not particularly useful.
We have automated tooling (ceph-release-notes, backport-resolve-issue) that try to parse the PR merge commit messages. Having multiple formats does make that job more difficult than it could be if only one format were used. Nathan
participants (4)
-
John Zachary Dover
-
Lenz Grimmer
-
Nathan Cutler
-
Patrick Donnelly