7 Sep
2020
7 Sep
'20
6:24 p.m.
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/#prope…
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
8 Sep
8 Sep
2:06 a.m.
On Mon, Sep 7, 2020 at 8:25 AM John Zachary Dover <zac.dover(a)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/#prope…
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
10:01 a.m.
On 9/8/20 1:06 AM, Patrick Donnelly wrote:
On Mon, Sep 7, 2020 at 8:25 AM John Zachary Dover
<zac.dover(a)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/#prope…
>
> 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(a)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(a)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(a)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(a)redhat.com>
Reviewed-by: Ernesto Puerta <epuertat(a)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)
6:27 p.m.
Hi Lenz,
On Tue, Sep 8, 2020 at 12:02 AM Lenz Grimmer <lgrimmer(a)suse.com> wrote:
I believe so.
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(a)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(a)redhat.com>
Reviewed-by: Ernesto Puerta <epuertat(a)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
6:47 p.m.
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.
1331
days inactive
1332
days old
4 comments
4 participants
participants (4)
-
John Zachary Dover -
Lenz Grimmer -
Nathan Cutler -
Patrick Donnelly