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

Create from template #2088

Closed
wants to merge 29 commits into from
Closed

Conversation

simkimsia
Copy link
Contributor

@simkimsia simkimsia commented Oct 21, 2021

This is to solve #1395 and #1974

With the last commit being 652bdcc

And this is a continuation of the PR in #2012

@harismuha123 I have tried to resolve merge conflicts and hope that @s-t-e-v-e-n-k can approve this PR.

isouza-daitan and others added 27 commits February 11, 2020 09:57
@simkimsia simkimsia mentioned this pull request Oct 21, 2021
Copy link
Collaborator

@s-t-e-v-e-n-k s-t-e-v-e-n-k left a comment

Choose a reason for hiding this comment

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

I've only lightly reviewed this, but it's full of unrelated changes that are making this very difficult to review, can you try and pull them out?

private=github.GithubObject.NotSet,
):
"""
:calls: `POST /repos/:template_owner/:template_repo/generate <https://developer.github.com/v3/repos/#create-repository-using-a-repository-template>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the old format and old URL for the docs.

@@ -456,18 +456,12 @@ def create_authorization(
isinstance(element, str) for element in scopes
), scopes
assert note is github.GithubObject.NotSet or isinstance(note, str), note
assert note_url is github.GithubObject.NotSet or isinstance(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did lint do this? These changes look unrelated.

), hireable
assert company is github.GithubObject.NotSet or isinstance(company, str), company
assert location is github.GithubObject.NotSet or isinstance(location, str), location
assert hireable is github.GithubObject.NotSet or isinstance(hireable, bool), hireable
Copy link
Collaborator

Choose a reason for hiding this comment

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

As above -- unrelated changes make this harder to review.

@simkimsia
Copy link
Contributor Author

Right sorry I will redo the PR

@simkimsia
Copy link
Contributor Author

Sorry I re-requested a review without realizing the latest commit doesn't directly compare with the main branch.

How do I do that?

Do I squash all my commits?

commit 6452ddf
Author: Steve Kowalik <steven@wedontsleep.org>
Date:   Sun Oct 24 15:21:31 2021 +1100

    Add Repository.rename_branch method (PyGithub#2089)

    The GitHub API exposes an endpoint to rename a branch, so we should
    support calling it. Sadly, there is not enough information to add
    that method to the Branch class, so expose it in the Repository object.

    Fixes PyGithub#1901

commit c8a945b
Author: Claire Johns <42869556+johnsc1@users.noreply.github.com>
Date:   Sun Oct 24 00:15:31 2021 -0400

    Add function to delete pending reviews on a pull request (PyGithub#1897)

    Add a delete method to PullRequestReview to allow dismissing them.

    Fixes PyGithub#1856

    Co-authored-by: bagashvilit <bagashvilit@allegheny.edu>
    Co-authored-by: WonjoonC <chos@allegheny.edu>

commit f1faf94
Author: Steve Kowalik <steven@wedontsleep.org>
Date:   Fri Oct 22 08:39:31 2021 +1100

    Cover all code paths in search_commits (PyGithub#2087)

    The search_commits method was only very lightly tested, meaning over
    half of it was not covered. Write another test case, covering all code
    paths.
@simkimsia
Copy link
Contributor Author

Sorry I think I made things worse by trying to solve the issue.
Now I have 891f450 which squashes a bunch of changes from main branch

I think i will refrain from pushing more commits for now. I think I have undo the various lint changes in 679858c but github seems to present them as changes based on the previous commit not against the main branch.

Happy to redo the whole thing if you can guide me. THank you

@simkimsia
Copy link
Contributor Author

simkimsia commented Oct 24, 2021

I am closing this PR and will redo at #2090

@simkimsia simkimsia closed this Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants