Skip to content

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}"

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