15 Dec
2020
15 Dec
'20
2:50 p.m.
Hi all,
After automating the per-component labelling in Pull Requests, this next PR
<https://github.com/ceph/ceph/pull/38548> is adding automation to the
`needs-rebase` label with the help of this Github Action
<https://github.com/marketplace/actions/label-conflicting-pull-requests>.
When the base branch (e.g.: master) of a PR is updated and the PR merge
results in conflicts:
- `needs-rebase` label will automatically be added.
- `ready-to-merge` label will be removed
- The following comment will be added: "This pull request can no longer
be automatically merged: a rebase is needed and changes have to be manually
resolved".
When the PR is rebased, conflict solved and re-pushed:
- The `needs-rebase` label will be automatically removed
(`ready-to-merge` label won't be added back though)
Kind Regards,
Ernesto Puerta
He / Him / His
Senior Software Engineer, Ceph
Red Hat <https://www.redhat.com/>
<https://www.redhat.com/>
15 Dec
15 Dec
6:57 p.m.
Hi Ernesto,
On Tue, Dec 15, 2020 at 3:21 AM Ernesto Puerta <epuertat(a)redhat.com> wrote:
Hi all,
After automating the per-component labelling in Pull Requests, this next PR is adding
automation to the `needs-rebase` label with the help of this Github Action.
When the base branch (e.g.: master) of a PR is updated and the PR merge results in
conflicts:
`needs-rebase` label will automatically be added.
`ready-to-merge` label will be removed
Are we already using this "ready-to-merge" label? What does it mean
exactly? That the PR is already QA'd and just needs merged or that the
PR has no conflicts? In any case, it does not seem useful to me and
adds visual clutter to the label list.
The following comment will be added: "This pull
request can no longer be automatically merged: a rebase is needed and changes have to be
manually resolved".
When the PR is rebased, conflict solved and re-pushed:
The `needs-rebase` label will be automatically removed (`ready-to-merge` label won't
be added back though)
Otherwise, a very useful change. Thanks for your efforts on it!
--
Patrick Donnelly, Ph.D.
He / Him / His
Principal Software Engineer
Red Hat Sunnyvale, CA
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D
8:26 p.m.
Hi Patrick,
Are we already using this "ready-to-merge" label? What does it mean
exactly? That the PR is already QA'd and just
needs merged or that the
PR has no conflicts? In any case, it does not seem useful to me and
adds visual clutter to the label list.
Until recently we used that in the dashboard for highlighting PRs meeting
all requirements (checks, reviews, QA, etc). Using a green label helped
visually identify PRs:
[image: image.png]
I'm not sure if our label colours are always helping understand the status
of a PR:
[image: image.png]
We recently stopped using the 'ready-to-merge' label and we now use a
column in our project board <https://github.com/ceph/ceph/projects/6>:
[image: image.png]
So I have no issue with removing the ready-to-merge label & the automation
rule in that PR. Is that ok?
The following comment will be added: "This pull
request can no longer be
automatically merged: a rebase is needed and changes have to be manually
resolved".
You're welcome!
Kind Regards,
Ernesto
When the PR is rebased, conflict solved and re-pushed:
The `needs-rebase` label will be automatically removed (`ready-to-merge`
label
won't be added back though)
Otherwise, a very useful change. Thanks for your efforts on it!
--
Patrick Donnelly, Ph.D.
He / Him / His
Principal Software Engineer
Red Hat Sunnyvale, CA
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D
8:31 p.m.
On Tue, Dec 15, 2020 at 11:57 AM Ernesto Puerta <epuertat(a)redhat.com> wrote:
+1 on the thanks!
You're welcome!
Hi Patrick,
Are we already using this "ready-to-merge" label? What does it mean
+1 for an RBD point-of-view. We go from "needs-qa" ->
"wip-XYZ-testing"
branch label -> to merged assuming the suite passes.
exactly? That the PR is already QA'd and just
needs merged or that the
PR has no conflicts? In any case, it does not seem useful to me and
adds visual clutter to the label list.
Until recently we used that in the dashboard for highlighting PRs meeting
all requirements (checks, reviews, QA, etc). Using a green label helped
visually identify PRs:
[image: image.png]
I'm not sure if our label colours are always helping understand the status
of a PR:
[image: image.png]
We recently stopped using the 'ready-to-merge' label and we now use a
column in our project board <https://github.com/ceph/ceph/projects/6>:
[image: image.png]
So I have no issue with removing the ready-to-merge label & the automation
rule in that PR. Is that ok?
The following
comment will be added: "This pull request can no longer be
automatically merged: a rebase is needed and changes have to be manually
resolved".
When the PR is rebased, conflict solved and re-pushed:
The `needs-rebase` label will be automatically removed
(`ready-to-merge` label
won't be added back though)
Otherwise, a very useful change. Thanks for your efforts on it!
Kind Regards,
Ernesto
--
Jason
--
Patrick Donnelly, Ph.D.
He / Him / His
Principal Software Engineer
Red Hat Sunnyvale, CA
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D
_______________________________________________
Dev mailing list -- dev(a)ceph.io
To unsubscribe send an email to dev-leave(a)ceph.io
10:03 p.m.
Removed the ready-to-merge label and the corresponding rule. Thank you for
the feedback!
Kind Regards,
Ernesto
On Tue, Dec 15, 2020 at 6:28 PM Nathan Cutler <ncutler(a)suse.com> wrote:
So I have no
issue with removing the ready-to-merge label
That one and "needs-backport" are the labels I most love to hate, so +1
from my
side as well.
Nathan
16 Dec
16 Dec
8:49 p.m.
This PR (https://github.com/ceph/ceph/pull/38548) was just merged. Please
let me know if you face any unexpected behaviour related with the
"needs-rebase" label setting/unsetting and the comment added.
Kind Regards,
Ernesto
1230
days inactive
1231
days old
6 comments
4 participants
participants (4)
-
Ernesto Puerta -
Jason Dillaman -
Nathan Cutler -
Patrick Donnelly