-
Notifications
You must be signed in to change notification settings - Fork 4
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
Changed so pr can now take in URL as well as pr number. #30
base: master
Are you sure you want to change the base?
Conversation
git-hub
Outdated
@@ -580,7 +560,10 @@ def cli(): | |||
@click.option('--label', '-l', default="", help="search PR by label") | |||
def hub(command, args, user, comment, number, branch, sort, opened, label): | |||
if command == "pr": | |||
pr_num = int(args[0]) | |||
if args[0].isdigit(): |
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.
What is this code doing?
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.
It checks if the argument supplied is an integer (so either the pr number(i.e. 4) or the url(e.g. #35)) so that it knows whether to take it as an integer or to find the pr number inside the url.
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.
It changed the url I inputted into my comment. But like if the user inputted instead of '30', they inputted 'https://github.com/machine-shop/oss_devkit/pull/ 30'
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.
OK, I suggest adding comments to both branches of the "if", saying what you're expecting in each branch. This will help future developers see what you're doing.
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.
added comments.
git-hub
Outdated
pr_num = int(args[0]) | ||
# if user inputs the url of the pull request | ||
else: | ||
pr_num = int(args[0].split('/')[-1]) |
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 can be slightly cleverer here, so that you can also paste in URLs like this:
https://github.com/machine-shop/oss_devkit/pull/30/files
We can do that by matching the URL to a regex, something that expects the form: https://github.com/(org_name)/(repo)/pull/(pr_nr)(/*)
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.
I changed it to using regex.
Changed to regex. |
Can now do "git hub pr 2" as well as "git hub pr {url for pr 2}"