Issue600

Title Convert patches to GitHub Pull Request (depends on issue586)
Priority feature Status in-progress
Superseder Nosy List anish.shah, berker.peksag, eric.snow, maciej.szulik
Assigned To Topics

Created on 2016-06-19.18:03:31 by anish.shah, last changed 2018-06-15.16:25:11 by eric.snow.

Files
File name Uploaded Type Edit Remove
patch-to-pr.diff anish.shah, 2016-06-19.18:03:30 text/plain
patch-to-pr.diff anish.shah, 2016-08-22.18:52:33 text/plain
Messages
msg3055 (view) Author: anish.shah Date: 2016-06-19.18:03:30
This is an initial patch to convert .patch/.diff files to GitHub Pull Requests.
msg3070 (view) Author: berker.peksag Date: 2016-07-13.17:40:04
How do you get the correct author information of the patch? I don't think there is an easy way [1] to extract the author information to use it in the pull requests.

[1] You could probably check the GitHub username field of the author but you could have to guess it anyway and I don't think we should guess it. People should convert their patches to pull requests manually.
msg3107 (view) Author: maciej.szulik Date: 2016-07-25.21:21:47
@Berker the work included in this issue is the main part of Anish work during his GSoC time. I'm currently in the process of reviewing his submissions.
msg3156 (view) Author: maciej.szulik Date: 2016-08-22.17:17:19
> endpoint = 'https://api.github.com/repos/AnishShah/cpython/pulls'

That endpoint doesn't look right, does it ;) ?

> "body": "fixes issue {}".format(issue_id),
...
> commit_msg = 'Fixes issue {} : {}'.format(issue_id, title)

It would be nice to use 'Fixes issue bpo<number>' in both cases, to be consistent
and so that patch from #589 links the issue properly, even if you're doing
this manually in the next lines.


> issue_pr = db.issue.get(issue_id, 'github_pullrequest_urls')

This needs update to reflect current state of #586, additionally you should
update the title to have that dependency in the title, like the others.


> # err takes stdout
> # if err:
> #     raise Exception(err)

We do need proper error handling, still.


> filename = db.file.get(file_id, 'name')
> content = db.file.get(file_id, 'content')
> fp = open(os.path.join(path, filename), 'wb')
> fp.write(content)
> fp.close()

Create a ContextManager (you need to write one manually) using tempfile.mkdtemp to create a temporary directory
and clean it afterwards.


> # git_workflow(newvalues)

If not needed - remove, if needed - uncomment. 


Would it be hard to add test cases covering that flow?
msg3157 (view) Author: anish.shah Date: 2016-08-22.18:46:34
I have updated the patch.

> We do need proper error handling, still.
The problem is "err" variable has stdout of that process. So, even if we change/push branch, it will be non-empty. Any idea how to solve this problem?

One more thing, if we create PR using API, it triggers GitHub webhooks. So, I'm removing the part where we are storing the response in DB.

Also, I'm not sure how to add tests for this.
msg3158 (view) Author: maciej.szulik Date: 2016-08-29.20:40:42
> I have updated the patch.

Thank you, the patch itself LGTM.

> > We do need proper error handling, still.
> The problem is "err" variable has stdout of that process. So, even if we change/push branch, it will be non-empty. Any idea how to solve this problem?

If I'm reading this [1] correctly communicate returns you a tuple where first argument
is stdout contents and second stderr, isn't that what you want?

> One more thing, if we create PR using API, it triggers GitHub webhooks. So, I'm removing the part where we are storing the response in DB.

Yup, your webhook should handle that part nicely, already.

> Also, I'm not sure how to add tests for this.

Yeah, let's leave it like this for now. 

[1] https://docs.python.org/2/library/subprocess.html#subprocess.Popen.communicate
msg3508 (view) Author: eric.snow Date: 2018-06-15.16:25:11
The main goal here is to make it easier for folks to move patches to GitHub.  So there are a number of things we can do to help, which mostly build on one another:

1. determine a git commit hash on which a patch applies cleanly
  * best guess based on date (i.e. from the patch's tracker metadata) or even by attempt [1]
  * it doesn't have to be perfect (folks can rebase, etc.)
2. add a column (in the "Files" table) for "Base Git Revision"
3. add a button/link (in its own column?) for "Create Git Branch"
  a. get necessary info (e.g. otherwise hidden form, pop-up, separate page)
    1. field for repo URL (pre-filled with generated GH one) [or use dedicated user/repo fields?]
      + use GH username associated with current user, if available
      + use "cpython" as default repo name
    2. field for branch name (pre-filled based on patch file name)
    3. field for base revision (blank? pre-filled with result of (1) above)
    4. field for commit message (pre-filled with something simple)
    5. check box for "create PR"
  b. apply the patch to a (local) git clone
    1. checkout the base revision
    2. create the branch
    3. apply the patch
    4. commit
  c. push the branch to the repo URL
  d. (optionally) create the PR
4. add a column (in the "Files" table) for links to PRs that have been created from the patch

(maybe) automate:

5. determine which patches on an issue should be turned into PRs
6. automate create PRs for groups of (manually/programatically) selected patches
7. (probably not) create PRs for all [valid (see (5) above)] patches on all open issues

Note that even just (2) would save folks a lot of hassle.  For older patches it's a real pain to figure out where the patch applies cleanly.


[1] If there isn't already a tool to help with this, there ought to be.  It would give a set of git hashes based on a timestamp (and hg repo).  If also given a patch, it could identify the newest one of those hashes to which the patch applies cleanly.
History
Date User Action Args
2018-06-15 16:25:11eric.snowsetnosy: + eric.snow
messages: + msg3508
2016-08-29 20:40:43maciej.szuliksetmessages: + msg3158
2016-08-22 18:52:33anish.shahsetfiles: + patch-to-pr.diff
2016-08-22 18:52:22anish.shahsetfiles: - patch-to-pr.diff
2016-08-22 18:46:35anish.shahsetfiles: + patch-to-pr.diff
messages: + msg3157
title: Convert patches to GitHub Pull Request -> Convert patches to GitHub Pull Request (depends on issue586)
2016-08-22 17:17:19maciej.szuliksetmessages: + msg3156
2016-07-25 21:21:47maciej.szuliksetmessages: + msg3107
2016-07-13 17:40:04berker.peksagsetnosy: + berker.peksag
messages: + msg3070
2016-06-19 18:03:31anish.shahcreate