Skip to content

Conversation

@iamvery
Copy link
Contributor

@iamvery iamvery commented Oct 4, 2013

I thought it might be useful to see the Github users you've added to your keys file. The idea is that if you pair a lot you may be making lots of changes to your keys file. At some point you think "gosh, what's even in this file anymore?"

What do you think? Is there anything else I need to do per project conventions that I missed?

Thanks for the great tool! 😄

* We're introducing a "list" method which requires no username argument
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is my only area of concern. I don't consider myself incredible with regular expressions, so I'm afraid this might be lacking. What do you think?

Copy link

Choose a reason for hiding this comment

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

I will often include a rubular link that shows how the regex works when I have to do things like this.

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 heard that suggestion on Ruby Rogues... great idea!

* This returns a list of only the github users which have been added to
  your keys file.
* This will list all the users you've added to your keys file with this
  gem.
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 this to the more liberal .+ because I realized Github allows "non-word" usernames (e.g. @abc-123). Also loved @wallace suggestion to link to the Rubular explanation! What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, yeah, good call 👍 I think we can be even more choosey here and just take non-whitespace characters. Thoughts? Something like:

# http://rubular.com/r/zXCkewmm0i
github\.com\/(\S+)

@chrishunt
Copy link
Owner

@iamvery Awesome, thanks much for making this happen. ✨ Sorry it took a couple weeks for me to have a look. Looking forward to merging this in. I left a few comments.

* This improves the "flow" of the examples by showing the use of `list`
  at the point in the example where two users have been added.

[chrishunt#14]
* Per the natural flow of the example:
  1. Add users
  2. List users
  3. Remove users
  4. List users

[chrishunt#14]
@iamvery
Copy link
Contributor Author

iamvery commented Oct 22, 2013

@chrishunt Yeah man! I'm stoked to help out 😄. No worries on the time. 👍

iamvery added a commit to iamvery/github-auth that referenced this pull request Oct 22, 2013
* There is a subtle difference in functionality, but it doesn't affect
  the implementation here.
* See https://github.com/chrishunt/github-auth/pull/14/files#r7122394
  for more information.

[chrishunt#14]
* There is a subtle difference in functionality, but it doesn't affect
  the implementation here.
* See chrishunt#14 (comment)
  for more information.

[chrishunt#14]
@iamvery
Copy link
Contributor Author

iamvery commented Oct 22, 2013

@chrishunt fixed up the weird explicit return in the spec helper. Also sorry about the duplicate reference above. I wanted to make sure I got a good link to the code comment for further explanation. Had for force-push an update to get the link right :/

chrishunt added a commit that referenced this pull request Oct 22, 2013
@chrishunt chrishunt merged commit d03fab3 into chrishunt:master Oct 22, 2013
@chrishunt
Copy link
Owner

@iamvery Thank you sir. ⚡

@chrishunt chrishunt mentioned this pull request Oct 22, 2013
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.

3 participants