Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set labels in cherry_picker.py #58

Closed
serhiy-storchaka opened this issue Mar 22, 2017 · 13 comments
Closed

Set labels in cherry_picker.py #58

serhiy-storchaka opened this issue Mar 22, 2017 · 13 comments

Comments

@serhiy-storchaka
Copy link
Member

Would be nice if cherry_picker.py set corresponding "cherry-pick for X.Y" labels and copy labels like "bugfix" or "documentation". It could also check if labels "needs backport to X.Y" are set.

@Mariatta
Copy link
Member

Yes, I'd like this too :)
I just don't (yet) know how to write the code to do this 😅

@brettcannon
Copy link
Member

Doing that will require using the GitHub API. If you want to go as far as having people securely store a GitHub auth token on their machine and use that, then you could even skip opening the browser and create the PR along with the appropriate labels, and even remove the "needs backport" label on the original PR.

You can use https://github.com/brettcannon/gidgethub for a library to talk to the GitHub API and https://developer.github.com/v3/ is the docs for the GitHub API itself. It actually just a standard REST API once you have a library to abstract out all of the GitHub-specific details (hence why I wrote gidgethub 😁 ).

But I think the key questions are (a) are you okay asking people to provide an auth token and saving it on disk somewhere appropriate and securely, and (b) if you go down this automated route do you want to continue to support the non-token version? One perk to doing this API solution is you will have implemented a decent chunk of the steps necessary for a bot (only missing pieces is handling a webhook to trigger the steps and adding a comment to the original PR when a cherry-pick fails).

@Mariatta
Copy link
Member

I'm quite interested in using gidgethub. I just installed it, and read the docs. I have a better idea now of how the labels can be set and unset.

Any suggestion of how to best store the token?
It's also going to be some manual work, right? Each user will have to first generate a token from within GitHub.

If not storing the token, what's the other option?

Thanks :)

@brettcannon
Copy link
Member

You can look at https://developer.github.com/v3/oauth/ if you want to try and get the token through a browser flow. Otherwise you should just link to the instructions on how to generate a GitHub auth token.

As for storing the token, that I don't have any suggestions on how best to do that. Maybe something like https://pypi.org/project/keyring/ ? Otherwise it could just go into a config file that has the proper permissions. I unfortunately don't know who best to ask on how to handle this. 😞

@Mariatta
Copy link
Member

Mariatta commented May 3, 2017

Thanks @brettcannon I've looked into storing the token using keyring, it doesn't seem to work when using venv. Maybe I'm doing it wrong... 😅

Anyway.. I'm trying to think about this from a different angle.
Maybe the bots like the-knights-who-say-ni or bedevere can apply the cherry-pick labels, similarly how it applies the CLA Signed label?
So when a PR is created, it can check if [X.Y] is in the PR title, and then apply cherry-pick for X.Y label.

  1. backports created with cherry_picker.py have [X.Y] for sure
  2. those not using cherry_picker.py have been instructed to do that in the devguide
  3. even when the cherry-picking bot is in place, I suppose it will also provide [X.Y] as the PR title. This might simplify the cherry-picking bot to just do the cherry-pick and PR creation, and not worry about applying labels.
  4. (non core-dev) contributors can still use cherry_picker.py and the labels still get applied without coredev's intervention.
  5. Backports using cherry_picker.py will also have the link to the original GitHub PR, (GH-XXX in the title/desc), the bot can remove the needs backport to [X.Y] label from the original PR (I think)

Or maybe I'm just complicating things 😛

@brettcannon
Copy link
Member

Having bedevere add the label based on the title format also works and SGTM (it won't be a Ni thing as that bot is strictly for the CLA to keep that code simple to avoid potentially botching something with legal ramifications).

@terryjreedy
Copy link
Member

If [x.y] in title leads to [cherry-picked] label, then I would have to know to not add [x.y] in label for patch that is not exactly a cherry pick. I am thinking of a recent default merge that altered about 12 files, including one idlelib file. I plan to backport just the change to the idlelib file. Should that get the label? Can cherry_picker.py selectively include parts of a patch? (That would be nice, as there should be more than one such situation.)

@brettcannon
Copy link
Member

@terryjreedy in your case it's still a backport of a PR, so I think the label makes sense as it ties to backporting work for the original PR.

As for partial backporting, what you will need to do is cherry-pick the commit without a commit and then revert the files you don't want changed (or copy the change over manually some other way).

@Mariatta
Copy link
Member

Mariatta commented May 3, 2017

Partial backporting will need to be done manually for now, but if we do #78 then it's possible :)

@terryjreedy
Copy link
Member

The particular issue I referenced is bpo-30166 and PR-1293, committed a few hours ago, which changed idlelib.pyshell. The Bootcamp "Backporting merged changes" and "17.4 Backporting ..." sections say nothing about manual backporting. Do I have to switch branches from master to 3.6 before creating a temporary backport branch? Or wait for someone to document how to work with worktree? Or wait for cherry-picker to be enhanced?

@Mariatta
Copy link
Member

Mariatta commented May 4, 2017

Sorry, that's my fault :( Manual backporting was documented in the DevGuide but I replaced it with cherry_picker.py. I can put it back if people want it. The readme in cherry_picker.py describes the manual process:

 #cd to cPython directory
$ git fetch upstream
$ git checkout -b <backport-branch-name> upstream/3.5. # if backporting to 3.5
$ git cherry-pick -x <commit hash>
# now you can test, or commit more changes
# possible to cherry-pick another commit here

$ git push origin <cherry-pick-branch>
$ git checkout master
$ git branch -D <cherry-pick-branch>

#go to GitHub and open the PR, choose 3.5 as the base branch

Repeat everything above for each branch.
Hope this helps :)
I've been working on the --no-push option, probably will get it done by end of the week.

@Mariatta
Copy link
Member

Mariatta commented May 5, 2017

@Mariatta
Copy link
Member

Closing this issue, because:

Thanks all :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants