-
Notifications
You must be signed in to change notification settings - Fork 4
Rename PR branch variables #32
base: master
Are you sure you want to change the base?
Conversation
Bug fix for the name of the base patch of the PR for the case where there are no formatting changes. When "origin/master" is specified as the base, the following error occurs: Traceback (most recent call last): File "/builds/Linaro/lkft/mirrors/skipfile-testing/squad-client-utils/read-skipfile-results", line 433, in <module> exit(run()) ^^^^^ File "/builds/Linaro/lkft/mirrors/skipfile-testing/squad-client-utils/read-skipfile-results", line 411, in run push_pr( File "/builds/Linaro/lkft/mirrors/skipfile-testing/squad-client-utils/read-skipfile-results", line 142, in push_pr pr = github_repo.create_pull( ^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.11/site-packages/github/Repository.py", line 1526, in create_pull headers, data = self._requester.requestJsonAndCheck("POST", f"{self.url}/pulls", input=post_parameters) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.11/site-packages/github/Requester.py", line 494, in requestJsonAndCheck return self.__check(*self.requestJson(verb, url, parameters, headers, input, self.__customConnection(url))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.11/site-packages/github/Requester.py", line 525, in __check raise self.createException(status, responseHeaders, data) github.GithubException.GithubException: 422 {"message": "Validation Failed", "errors": [{"resource": "PullRequest", "field": "base", "code": "invalid"}], "documentation_url": "https://docs.github.com/rest/pulls/pulls#create-a-pull-request"} The fix is to change the base patch location from "origin/master" to "master". Signed-off-by: Katie Worton <[email protected]>
Rename `base` to `pr_base_patch` and `base_patch` to `next_pr_base_patch` to give more descriptive names for these variables. Signed-off-by: Katie Worton <[email protected]>
@@ -218,15 +218,15 @@ def run(raw_args=None): | |||
github_repo = g.get_repo(f"{username}/{repo_name}") | |||
|
|||
# Base patch - base patch for PRs | |||
base_patch = "origin/master" | |||
next_pr_base_patch = "master" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
next_base_branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should say:
base_branch = next_base_branch = "master"
|
||
# If there are any formatting updates, make these | ||
if repo.index.diff("HEAD"): | ||
summary = "Skipfile formatting updates" | ||
message = "Clean up skipfile formatting." | ||
base = "master" | ||
pr_base_patch = next_pr_base_patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we don't need this or?
@@ -249,7 +249,7 @@ def run(raw_args=None): | |||
github_repo, | |||
summary, | |||
message, | |||
base, | |||
pr_base_patch, | |||
head, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we use head here and not the next_pr_base_patch variable ?
@@ -414,7 +415,7 @@ def run(raw_args=None): | |||
github_repo, | |||
summary, | |||
message, | |||
base, | |||
pr_base_patch, | |||
head, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
@@ -423,7 +424,7 @@ def run(raw_args=None): | |||
new_skipitem = None | |||
# Log commit to file | |||
with open(f"{head}.diff", "w") as file: | |||
diff = popen(f"git diff {base} {head}").read() | |||
diff = popen(f"git diff {pr_base_patch} {head}").read() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
head is used here too, should it or ?
Rename the variables for holding the base patch info so it is easier to differentiate them and understand their purpose.