ptl-tool: load labeled PRs #18231

batrick · 2017-10-10T23:57:33Z

This lets you label in GitHub the PRs you want to test/merge.

Signed-off-by: Patrick Donnelly pdonnell@redhat.com

batrick · 2017-10-10T23:58:02Z

@liewegas this is that feature you really wanted: getting PRs that you've previously labeled for merging/testing.

joscollin · 2017-10-11T04:08:32Z

src/script/ptl-tool.py

+# Adding labeled PR #18192 to PR list
+# Will merge PRs: [18192]
+# Merging PR #18192
+# Leaving HEAD detached; no branch anchors your commit


I think we can add this simple example too:

src/script/ptl-tool.py --base master --merge-branch <wip-test-branch> --pr-label wip-<name>-testing

I'm trying not to have too many examples. :)

joscollin · 2017-10-11T04:11:36Z

The initial test looks fine. But I will do more testing :-)

joscollin · 2017-10-11T04:22:46Z

src/script/ptl-tool.py

+    prs = args.prs
+    if args.pr_label:
+        payload = {'labels': args.pr_label, 'sort': 'created', 'direction': 'desc'}
+        labeled_prs = requests.get("https://api.github.com/repos/ceph/ceph/issues", auth=(USER, PASSWORD), params=payload)


See, created the test branch for an invalid pr-label.

$ src/script/ptl-tool.py --base master --merge-branch wip-jcollin-testing --pr-label wip-jcollin-testing2 #or wip-aoeu-testing Detaching HEAD onto base: master Will merge PRs: [] Checked out new branch wip-jcollin-testing-20171011 Created tag testing/wip-jcollin-testing-20171011_04 jcollin@smithi042:~/test-ptl-oct11$ git branch master * wip-jcollin-testing-20171011

Please check if the branch exists first and drop it. May be move the existing label validation code to a function ?

--pr-label ' 'is bypassing it. I have pressed Ctrl+C and it saved me :-D.

jcollin@smithi042:~/test-ptl-oct12$ src/script/ptl-tool.py --base master --merge-branch wip-jcollin-testing --pr-label ' ' Adding labeled PR #18271 to PR list Adding labeled PR #18270 to PR list Adding labeled PR #18269 to PR list Adding labeled PR #18268 to PR list Adding labeled PR #18267 to PR list Adding labeled PR #18266 to PR list Adding labeled PR #18265 to PR list Adding labeled PR #18264 to PR list Adding labeled PR #18263 to PR list Adding labeled PR #18262 to PR list Adding labeled PR #18261 to PR list Adding labeled PR #18260 to PR list Adding labeled PR #18259 to PR list Adding labeled PR #18257 to PR list Adding labeled PR #18256 to PR list Adding labeled PR #18255 to PR list Adding labeled PR #18254 to PR list Adding labeled PR #18253 to PR list Adding labeled PR #18251 to PR list Adding labeled PR #18250 to PR list Adding labeled PR #18249 to PR list Adding labeled PR #18248 to PR list Adding labeled PR #18247 to PR list Adding labeled PR #18246 to PR list Adding labeled PR #18245 to PR list Adding labeled PR #18244 to PR list Adding labeled PR #18243 to PR list Adding labeled PR #18241 to PR list Adding labeled PR #18240 to PR list Adding labeled PR #18239 to PR list Will merge PRs: [18271, 18270, 18269, 18268, 18267, 18266, 18265, 18264, 18263, 18262, 18261, 18260, 18259, 18257, 18256, 18255, 18254, 18253, 18251, 18250, 18249, 18248, 18247, 18246, 18245, 18244, 18243, 18241, 18240, 18239] Detaching HEAD onto base: master Merging PR #18271 Labeled PR #18271 wip-jcollin-testing Merging PR #18270 ^CTraceback (most recent call last): File "src/script/ptl-tool.py", line 308, in <module> File "src/script/ptl-tool.py", line 305, in main

okay, fixed

Check the empty string also.

$ src/script/ptl-tool.py --base master --merge-branch wip-jcollin-testing --pr-label '' Will merge PRs: [] Detaching HEAD onto base: master Checked out new branch wip-jcollin-testing-20171013 Created tag testing/wip-jcollin-testing-20171013

Adding labeled PR #18284 to PR list

Do we need to add and merge an unreviewed / unapproved PR for testing ? Do we need to check the PR status before the add / merge ?

Check the empty string also.

done.

Do we need to add and merge an unreviewed / unapproved PR for testing ? Do we need to check the PR status before the add / merge ?

No, that should stay the ptl's responsibility to check.

joscollin · 2017-10-16T06:51:16Z

@batrick I did this to merge wip-ptltool1-testing PRs to my testing branch:
$src/script/ptl-tool.py --base master --merge-branch wip-jcollin-testing --pr-label wip-ptltool1-testing

Then I want to merge wip-ptltool2-testing also to the same testing branch. So I did:

$ src/script/ptl-tool.py --base-path refs/heads/ --base wip-jcollin-testing-20171016 --branch wip-jcollin-testing-20171016 wip-ptltool2-testing
usage: ptl-tool.py [-h] [--branch BRANCH]
                   [--merge-branch-name MERGE_BRANCH_NAME] [--base BASE]
                   [--base-path BASE_PATH] [--git-dir GIT] [--label LABEL]
                   [--pr-label PR_LABEL]
                   [PR [PR ...]]
ptl-tool.py: error: argument PR: invalid int value: 'wip-ptltool2-testing'

Is this expected ? This is working as expected, if I try with the PR number:
$ src/script/ptl-tool.py --base-path refs/heads/ --base wip-jcollin-testing-20171016 --branch wip-jcollin-testing-20171016 18107

But not with the label feature.

This lets you label in GitHub the PRs you want to test/merge. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick · 2017-10-17T04:19:13Z

@batrick I did this to merge wip-ptltool1-testing PRs to my testing branch:
? $src/script/ptl-tool.py --base master --merge-branch wip-jcollin-testing --pr-label wip-ptltool1-testing

There is no --merge-branch option.

Then I want to merge wip-ptltool2-testing also to the same testing branch. So I did:

$ src/script/ptl-tool.py --base-path refs/heads/ --base wip-jcollin-testing-20171016 --branch wip-jcollin-testing-20171016 wip-ptltool2-testing
usage: ptl-tool.py [-h] [--branch BRANCH]
                   [--merge-branch-name MERGE_BRANCH_NAME] [--base BASE]
                   [--base-path BASE_PATH] [--git-dir GIT] [--label LABEL]
                   [--pr-label PR_LABEL]
                   [PR [PR ...]]
ptl-tool.py: error: argument PR: invalid int value: 'wip-ptltool2-testing'

Looks like you forgot --pr-label wip-ptltool2-testing?

joscollin · 2017-10-17T04:43:47Z

@batrick Yes, I missed --pr-label. Sorry for the negligence, I didn't even check the error message.

joscollin

@batrick Now it looks like your original example doesn't create a test branch. It leaves the HEAD detached, but no testing branch created.

I have used:
$ src/script/ptl-tool.py --base master --branch HEAD --merge-branch-name master --label - --pr-label wip-ptltool1-testing

Could you please check that ?

joscollin · 2017-10-17T05:44:34Z

@batrick Oh, that is intended.

leaving a detached HEAD (i.e. do not update your repo's master branch) and do not label

joscollin

@batrick Looks good to me. I think people can start using the script now, while we continue working on it.

@liewegas @yuriw Do you have any suggestions on this PR ? If nothing at the moment, we could merge this.

joscollin · 2017-10-17T14:11:30Z

Merged as this is an independent script (doesn't break anything else).

batrick added core needs-review labels Oct 10, 2017

batrick requested review from liewegas and joscollin October 10, 2017 23:57

joscollin added the wip-jcollin-testing label Oct 11, 2017

joscollin requested changes Oct 11, 2017

View reviewed changes

joscollin removed the needs-review label Oct 11, 2017

batrick force-pushed the ptl-tool-labeled-prs branch 3 times, most recently from 6a3f1a5 to 38a9e53 Compare October 14, 2017 00:52

ptl-tool: load labeled PRs

9978659

This lets you label in GitHub the PRs you want to test/merge. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick force-pushed the ptl-tool-labeled-prs branch from 38a9e53 to 9978659 Compare October 17, 2017 04:17

joscollin requested changes Oct 17, 2017

View reviewed changes

joscollin approved these changes Oct 17, 2017

View reviewed changes

joscollin merged commit 24dacf5 into ceph:master Oct 17, 2017

batrick deleted the ptl-tool-labeled-prs branch October 25, 2017 13:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ptl-tool: load labeled PRs #18231

ptl-tool: load labeled PRs #18231

batrick commented Oct 10, 2017

batrick commented Oct 10, 2017

joscollin Oct 11, 2017

batrick Oct 11, 2017

joscollin commented Oct 11, 2017

joscollin Oct 11, 2017

batrick Oct 11, 2017

joscollin Oct 12, 2017

batrick Oct 12, 2017

joscollin Oct 13, 2017 •

edited

batrick Oct 14, 2017 •

edited by joscollin

joscollin commented Oct 16, 2017

batrick commented Oct 17, 2017

joscollin commented Oct 17, 2017

joscollin left a comment

joscollin commented Oct 17, 2017 •

edited

joscollin left a comment

joscollin commented Oct 17, 2017

ptl-tool: load labeled PRs #18231

ptl-tool: load labeled PRs #18231

Conversation

batrick commented Oct 10, 2017

batrick commented Oct 10, 2017

joscollin Oct 11, 2017

Choose a reason for hiding this comment

batrick Oct 11, 2017

Choose a reason for hiding this comment

joscollin commented Oct 11, 2017

joscollin Oct 11, 2017

Choose a reason for hiding this comment

batrick Oct 11, 2017

Choose a reason for hiding this comment

joscollin Oct 12, 2017

Choose a reason for hiding this comment

batrick Oct 12, 2017

Choose a reason for hiding this comment

joscollin Oct 13, 2017 • edited

Choose a reason for hiding this comment

batrick Oct 14, 2017 • edited by joscollin

Choose a reason for hiding this comment

joscollin commented Oct 16, 2017

batrick commented Oct 17, 2017

joscollin commented Oct 17, 2017

joscollin left a comment

Choose a reason for hiding this comment

joscollin commented Oct 17, 2017 • edited

joscollin left a comment

Choose a reason for hiding this comment

joscollin commented Oct 17, 2017

joscollin Oct 13, 2017 •

edited

batrick Oct 14, 2017 •

edited by joscollin

joscollin commented Oct 17, 2017 •

edited