Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.

Rename PR branch variables #32

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

katieworton
Copy link
Member

Rename the variables for holding the base patch info so it is easier to differentiate them and understand their purpose.

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]>
@katieworton katieworton mentioned this pull request Oct 9, 2023
@@ -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"
Copy link
Collaborator

Choose a reason for hiding this comment

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

next_base_branch?

Copy link
Collaborator

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
Copy link
Collaborator

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,
Copy link
Collaborator

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,
Copy link
Collaborator

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()
Copy link
Collaborator

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 ?

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

Successfully merging this pull request may close these issues.

2 participants