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

Add --label argument to label new PRs #65

Closed
wants to merge 7 commits into from
Closed

Add --label argument to label new PRs #65

wants to merge 7 commits into from

Conversation

samestep
Copy link
Collaborator

@samestep samestep commented Jun 9, 2021

Resolves #64 (opened by @SplitInfinity).

@samestep samestep requested a review from ezyang June 9, 2021 23:01
ghstack/github_fake.py Outdated Show resolved Hide resolved
test_ghstack.py Outdated Show resolved Hide resolved
@ezyang
Copy link
Owner

ezyang commented Jun 10, 2021

Solid, thanks for extending the test infra too with this change!

@samestep
Copy link
Collaborator Author

extending the test infra

Actually, I did have a question about this: I just added a labels property to the PullRequest class, but I wasn't sure if that was the right thing to do since I couldn't find that in the GraphQL schema we use; is that OK?

@ezyang
Copy link
Owner

ezyang commented Jun 10, 2021

Actually, I did have a question about this: I just added a labels property to the PullRequest class, but I wasn't sure if that was the right thing to do since I couldn't find that in the GraphQL schema we use; is that OK?

Oh it's probably in the Issue class (every PullRequest is an Issue). Do you see it now? Labels are modeled in a sort of complicated way but it would be good to do it properly.

@samestep
Copy link
Collaborator Author

samestep commented Jun 14, 2021

@ezyang I updated this PR to fix the GraphQL test infra for labels (see also grantjenks/python-sortedcontainers#107 for more context on some of those changes), and changed it so now it applies the --labels to all PRs in the stack instead of just the new ones; asking for review again

@samestep samestep requested a review from ezyang June 14, 2021 20:49
test_ghstack.py Outdated Show resolved Hide resolved
test_ghstack.py Outdated Show resolved Hide resolved
@samestep samestep requested a review from ezyang June 15, 2021 16:19
@samestep
Copy link
Collaborator Author

@ezyang I fixed the mutable argument default and simplified the test_labels helper function

@ezyang
Copy link
Owner

ezyang commented Jun 15, 2021

@samestep and I chatted about this PR and we agreed to park it for now while waiting for more details about optionally triggered CI builds to pan out. The label GraphQL changes are generally useful and we should make sure they eventually get merged, even if we don't actually have the --label argument.

@samestep
Copy link
Collaborator Author

Closing this for now since there aren't currently plans to merge it.

@samestep samestep closed this Aug 16, 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.

Add command line option to specify labels
3 participants