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

Changed so pr can now take in URL as well as pr number. #30

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

Conversation

ttran428
Copy link
Contributor

@ttran428 ttran428 commented Apr 5, 2018

Can now do "git hub pr 2" as well as "git hub pr {url for pr 2}"

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

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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'

Copy link
Contributor

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.

Copy link
Contributor Author

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])
Copy link
Contributor

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)(/*)

Copy link
Contributor Author

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.

@ttran428
Copy link
Contributor Author

ttran428 commented Jun 3, 2018

Changed to regex.

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.

2 participants