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

Allow for deleting and restoring branch associated with PR #1784

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
28 changes: 28 additions & 0 deletions github/PullRequest.py
Original file line number Diff line number Diff line change
Expand Up @@ -835,12 +835,37 @@ def is_merged(self):
status, headers, data = self._requester.requestJson("GET", self.url + "/merge")
return status == 204

def restore_branch(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing .pyi file API addition

"""
:meth:`Repository.create_git_ref`
austinsasko marked this conversation as resolved.
Show resolved Hide resolved
:rtype: :class:`github.GitRef.GitRef`
"""
return self.head.repo.create_git_ref(
f"refs/heads/{self.head.ref}", sha=self.head.sha
)

def delete_branch(self, force=False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing .pyi file API addition

"""
:meth:`GitRef.delete`
austinsasko marked this conversation as resolved.
Show resolved Hide resolved
:rtype: bool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some notes on the semantics of force.

"""
if not force:
remaining_pulls = self.head.repo.get_pulls(head=self.head.ref)
if remaining_pulls.totalCount > 0:
raise AttributeError(
"PRs referencing this branch remain. Not deleting the branch"
)
Comment on lines +855 to +857
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not call this AttributeError, because the values of the attributes are right, This is more a RuntimeError, as delete_branch(force=False) should not be called as long as there exist pull requests.

Suggested change
raise AttributeError(
"PRs referencing this branch remain. Not deleting the branch"
)
raise RuntimeError(
"This branch is referenced by open pull requests, set force=True to delete this branch."
)

else:
austinsasko marked this conversation as resolved.
Show resolved Hide resolved
return self.head.repo.get_git_ref(f"heads/{self.head.ref}").delete()
return self.head.repo.get_git_ref(f"heads/{self.head.ref}").delete()

def merge(
self,
commit_message=github.GithubObject.NotSet,
commit_title=github.GithubObject.NotSet,
merge_method=github.GithubObject.NotSet,
sha=github.GithubObject.NotSet,
deletebranch=False,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing .pyi file API modification

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
deletebranch=False,
delete_branch=False,

):
"""
:calls: `PUT /repos/:owner/:repo/pulls/:number/merge <http://developer.github.com/v3/pulls>`_
Expand Down Expand Up @@ -872,6 +897,9 @@ def merge(
headers, data = self._requester.requestJsonAndCheck(
"PUT", self.url + "/merge", input=post_parameters
)
if deletebranch:
self.delete_branch()

Comment on lines +898 to +900
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I don't like adding this side-effect here. Given we have delete_branch, it is very easy for users to delete the branch with a single line after merge returned. When more pull requests remain, delete_branch raises an error and call site does not get the successful PullRequestMergeStatus response.

return github.PullRequestMergeStatus.PullRequestMergeStatus(
self._requester, headers, data, completed=True
)
Expand Down
72 changes: 72 additions & 0 deletions tests/PullRequest.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@

import datetime

import github
austinsasko marked this conversation as resolved.
Show resolved Hide resolved

from . import Framework


Expand All @@ -49,6 +51,9 @@ def setUp(self):
flo_repo = self.g.get_repo("FlorentClarret/PyGithub")
self.pullMaintainerCanModify = flo_repo.get_pull(2)

self.delete_restore_repo = self.g.get_repo("austinsasko/PyGithub")
self.delete_restore_pull = self.delete_restore_repo.get_pull(21)

def testAttributesIssue256(self):
self.assertEqual(
self.pullIssue256Closed.closed_at,
Expand Down Expand Up @@ -402,3 +407,70 @@ def testUpdateBranch(self):
self.pull.update_branch("addaebea821105cf6600441f05ff2b413ab21a36")
)
self.assertTrue(self.pull.update_branch())

def testDeleteOnMerge(self):
self.assertTrue(
self.delete_restore_repo.get_branch(self.delete_restore_pull.head.ref)
)
self.assertFalse(self.delete_restore_pull.is_merged())
status = self.delete_restore_pull.merge(deletebranch=True)
self.assertTrue(status.merged)
self.assertTrue(self.delete_restore_pull.is_merged())
with self.assertRaises(github.GithubException) as raisedexp:
self.delete_restore_repo.get_branch(self.delete_restore_pull.head.ref)
self.assertEqual(raisedexp.exception.status, 404)
self.assertEqual(
austinsasko marked this conversation as resolved.
Show resolved Hide resolved
raisedexp.exception.data,
{
"documentation_url": "https://docs.github.com/rest/reference/repos#get-a-branch",
"message": "Branch not found",
},
)

def testRestoreBranch(self):
with self.assertRaises(github.GithubException) as raisedexp:
self.delete_restore_repo.get_branch(self.delete_restore_pull.head.ref)
self.assertEqual(raisedexp.exception.status, 404)
self.assertEqual(
raisedexp.exception.data,
{
"documentation_url": "https://docs.github.com/rest/reference/repos#get-a-branch",
"message": "Branch not found",
},
)
self.assertTrue(self.delete_restore_pull.restore_branch())
self.assertTrue(
self.delete_restore_repo.get_branch(self.delete_restore_pull.head.ref)
)

def testDeleteBranch(self):
self.assertTrue(
self.delete_restore_repo.get_branch(self.delete_restore_pull.head.ref)
)
self.assertIs(self.delete_restore_pull.delete_branch(force=False), None)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this assertion?

Suggested change
self.assertIs(self.delete_restore_pull.delete_branch(force=False), None)
self.delete_restore_pull.delete_branch(force=False)

with self.assertRaises(github.GithubException) as raisedexp:
self.delete_restore_repo.get_branch(self.delete_restore_pull.head.ref)
self.assertEqual(raisedexp.exception.status, 404)
self.assertEqual(
raisedexp.exception.data,
{
"documentation_url": "https://docs.github.com/rest/reference/repos#get-a-branch",
"message": "Branch not found",
},
)

def testForceDeleteBranch(self):
austinsasko marked this conversation as resolved.
Show resolved Hide resolved
self.assertTrue(
self.delete_restore_repo.get_branch(self.delete_restore_pull.head.ref)
)
self.assertEqual(self.delete_restore_pull.delete_branch(force=True), None)
with self.assertRaises(github.GithubException) as raisedexp:
self.delete_restore_repo.get_branch(self.delete_restore_pull.head.ref)
self.assertEqual(raisedexp.exception.status, 404)
self.assertEqual(
raisedexp.exception.data,
{
"documentation_url": "https://docs.github.com/rest/reference/repos#get-a-branch",
"message": "Branch not found",
},
)
22 changes: 22 additions & 0 deletions tests/ReplayData/PullRequest.setUp.txt

Large diffs are not rendered by default.

55 changes: 55 additions & 0 deletions tests/ReplayData/PullRequest.testDeleteBranch.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
https
GET
api.github.com
None
/repos/austinsasko/PyGithub/branches/revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
200
[('Server', 'GitHub.com'), ('Date', 'Fri, 11 Dec 2020 22:17:38 GMT'), ('Content-Type', 'application/json; charset=utf-8'), ('Transfer-Encoding', 'chunked'), ('Status', '200 OK'), ('Cache-Control', 'private, max-age=60, s-maxage=60'), ('Vary', 'Accept, Authorization, Cookie, X-GitHub-OTP, Accept-Encoding, Accept, X-Requested-With'), ('ETag', 'W/"9e7706dfd919e04e9df872052add55475df9953d3b9c0d20df34fdeb8effc446"'), ('X-OAuth-Scopes', 'public_repo, read:user, repo:status'), ('X-Accepted-OAuth-Scopes', ''), ('X-GitHub-Media-Type', 'github.v3; format=json'), ('X-RateLimit-Limit', '5000'), ('X-RateLimit-Remaining', '4971'), ('X-RateLimit-Reset', '1607728588'), ('X-RateLimit-Used', '29'), ('Access-Control-Expose-Headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset'), ('Access-Control-Allow-Origin', '*'), ('Strict-Transport-Security', 'max-age=31536000; includeSubdomains; preload'), ('X-Frame-Options', 'deny'), ('X-Content-Type-Options', 'nosniff'), ('X-XSS-Protection', '1; mode=block'), ('Referrer-Policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('Content-Security-Policy', "default-src 'none'"), ('Content-Encoding', 'gzip'), ('X-GitHub-Request-Id', 'FFDC:379D:155E19:322526:5FD3F001')]
{"name":"revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing","_links":{"self":"https://api.github.com/repos/austinsasko/PyGithub/branches/revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing","html":"https://github.com/austinsasko/PyGithub/tree/revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing"},"protected":false,"protection":{"enabled":false,"required_status_checks":{"enforcement_level":"off","contexts":[]}},"protection_url":"https://api.github.com/repos/austinsasko/PyGithub/branches/revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing/protection"}

https
GET
api.github.com
None
/repos/austinsasko/PyGithub/pulls?head=revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing&per_page=1
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
200
[('Server', 'GitHub.com'), ('Date', 'Fri, 11 Dec 2020 22:17:38 GMT'), ('Content-Type', 'application/json; charset=utf-8'), ('Content-Length', '2'), ('Status', '200 OK'), ('Cache-Control', 'private, max-age=60, s-maxage=60'), ('Vary', 'Accept, Authorization, Cookie, X-GitHub-OTP, Accept-Encoding, Accept, X-Requested-With'), ('ETag', '"5b0893a1da6bb8cfc57acf8d3216d8751325252d4a5750cede94ca1dd53c6a54"'), ('X-OAuth-Scopes', 'public_repo, read:user, repo:status'), ('X-Accepted-OAuth-Scopes', ''), ('X-GitHub-Media-Type', 'github.v3; format=json'), ('X-RateLimit-Limit', '5000'), ('X-RateLimit-Remaining', '4970'), ('X-RateLimit-Reset', '1607728588'), ('X-RateLimit-Used', '30'), ('Access-Control-Expose-Headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset'), ('Access-Control-Allow-Origin', '*'), ('Strict-Transport-Security', 'max-age=31536000; includeSubdomains; preload'), ('X-Frame-Options', 'deny'), ('X-Content-Type-Options', 'nosniff'), ('X-XSS-Protection', '1; mode=block'), ('Referrer-Policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('Content-Security-Policy', "default-src 'none'"), ('X-GitHub-Request-Id', 'FFDD:43CF:26E97C:578F6F:5FD3F002')]
[]

https
GET
api.github.com
None
/repos/austinsasko/PyGithub/git/refs/heads/revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
200
[('Server', 'GitHub.com'), ('Date', 'Fri, 11 Dec 2020 22:17:38 GMT'), ('Content-Type', 'application/json; charset=utf-8'), ('Transfer-Encoding', 'chunked'), ('Status', '200 OK'), ('Cache-Control', 'private, max-age=60, s-maxage=60'), ('Vary', 'Accept, Authorization, Cookie, X-GitHub-OTP, Accept-Encoding, Accept, X-Requested-With'), ('ETag', 'W/"50d4d8f24500a45face48f0774778f2df511639264b29745fc3a3dd0880943a6"'), ('Last-Modified', 'Fri, 11 Dec 2020 22:17:23 GMT'), ('X-Poll-Interval', '300'), ('X-OAuth-Scopes', 'public_repo, read:user, repo:status'), ('X-Accepted-OAuth-Scopes', 'repo'), ('X-GitHub-Media-Type', 'github.v3; format=json'), ('X-RateLimit-Limit', '5000'), ('X-RateLimit-Remaining', '4969'), ('X-RateLimit-Reset', '1607728588'), ('X-RateLimit-Used', '31'), ('Access-Control-Expose-Headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset'), ('Access-Control-Allow-Origin', '*'), ('Strict-Transport-Security', 'max-age=31536000; includeSubdomains; preload'), ('X-Frame-Options', 'deny'), ('X-Content-Type-Options', 'nosniff'), ('X-XSS-Protection', '1; mode=block'), ('Referrer-Policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('Content-Security-Policy', "default-src 'none'"), ('Content-Encoding', 'gzip'), ('X-GitHub-Request-Id', 'FFDE:5324:70192:13F750:5FD3F002')]
{"ref":"refs/heads/revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing","node_id":"MDM6UmVmMzIwNjYxMzYwOnJlZnMvaGVhZHMvcmV2ZXJ0LTIwLXJldmVydC0xOS1yZXZlcnQtMTgtcmV2ZXJ0LTE2LXJldmVydC0xNS1yZXZlcnQtMTQtcmV2ZXJ0LTEzLXJldmVydC0xMi1yZXZlcnQtMTEtcmV2ZXJ0LTEwLXJldmVydC05LXJldmVydC04LXJldmVydC03LXJldmVydC02LXJldmVydC01LXJldmVydC00LXJldmVydC0zLXJldmVydC0yLXJldmVydC0xLXRlc3Rpbmc=","url":"https://api.github.com/repos/austinsasko/PyGithub/git/refs/heads/revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing","object":{"sha":"adf76c80c6d28dc2d47b1a22622082786192c887","type":"commit","url":"https://api.github.com/repos/austinsasko/PyGithub/git/commits/adf76c80c6d28dc2d47b1a22622082786192c887"}}

https
DELETE
api.github.com
None
/repos/austinsasko/PyGithub/git/refs/heads/revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
204
[('Server', 'GitHub.com'), ('Date', 'Fri, 11 Dec 2020 22:17:38 GMT'), ('Status', '204 No Content'), ('X-OAuth-Scopes', 'public_repo, read:user, repo:status'), ('X-Accepted-OAuth-Scopes', 'repo'), ('X-GitHub-Media-Type', 'github.v3; format=json'), ('X-RateLimit-Limit', '5000'), ('X-RateLimit-Remaining', '4968'), ('X-RateLimit-Reset', '1607728588'), ('X-RateLimit-Used', '32'), ('Access-Control-Expose-Headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset'), ('Access-Control-Allow-Origin', '*'), ('Strict-Transport-Security', 'max-age=31536000; includeSubdomains; preload'), ('X-Frame-Options', 'deny'), ('X-Content-Type-Options', 'nosniff'), ('X-XSS-Protection', '1; mode=block'), ('Referrer-Policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('Content-Security-Policy', "default-src 'none'"), ('Vary', 'Accept-Encoding, Accept, X-Requested-With'), ('X-GitHub-Request-Id', 'FFDF:110B:3EC696:68A9AE:5FD3F002')]


https
GET
api.github.com
None
/repos/austinsasko/PyGithub/branches/revert-20-revert-19-revert-18-revert-16-revert-15-revert-14-revert-13-revert-12-revert-11-revert-10-revert-9-revert-8-revert-7-revert-6-revert-5-revert-4-revert-3-revert-2-revert-1-testing
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
404
[('Server', 'GitHub.com'), ('Date', 'Fri, 11 Dec 2020 22:17:39 GMT'), ('Content-Type', 'application/json; charset=utf-8'), ('Transfer-Encoding', 'chunked'), ('Status', '404 Not Found'), ('X-OAuth-Scopes', 'public_repo, read:user, repo:status'), ('X-Accepted-OAuth-Scopes', ''), ('X-GitHub-Media-Type', 'github.v3; format=json'), ('X-RateLimit-Limit', '5000'), ('X-RateLimit-Remaining', '4967'), ('X-RateLimit-Reset', '1607728588'), ('X-RateLimit-Used', '33'), ('Access-Control-Expose-Headers', 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset'), ('Access-Control-Allow-Origin', '*'), ('Strict-Transport-Security', 'max-age=31536000; includeSubdomains; preload'), ('X-Frame-Options', 'deny'), ('X-Content-Type-Options', 'nosniff'), ('X-XSS-Protection', '1; mode=block'), ('Referrer-Policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'), ('Content-Security-Policy', "default-src 'none'"), ('Vary', 'Accept-Encoding, Accept, X-Requested-With'), ('Content-Encoding', 'gzip'), ('X-GitHub-Request-Id', 'FFE0:5E4B:63459:11BB67:5FD3F002')]
{"message":"Branch not found","documentation_url":"https://docs.github.com/rest/reference/repos#get-a-branch"}