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

Only use the GITHUB_API_TOKEN if it's not empty #231

Closed
wants to merge 4 commits into from
Closed

Only use the GITHUB_API_TOKEN if it's not empty #231

wants to merge 4 commits into from

Conversation

petems
Copy link
Contributor

@petems petems commented Jun 13, 2014

Unfortunately the Ruby ENV['foo'] returns true regardless or not if the value is empty or not.

I was having some issues with a terminal session where I purposefully set all env variables to blank for when I'm screen sharing and then puppet-librarian kept failing with a 401. Looking it up it was because it was calling https://api.github.com/foo?page=1&per_page=100?access_token=

I put in a Puts to alert the user, but can remove if needed

petems added 2 commits June 13, 2014 18:43
Also, for consistency use ENV[TOKEN_KEY] across the board.
There's probably a better way of mocking environmental variables...
@carlossg
Copy link
Collaborator

Thanks, could you use debug { "..." } instead of the puts ?
also, it'd be nice to avoid the code duplication and create a new method to append the api token to any url

@petems
Copy link
Contributor Author

petems commented Jun 15, 2014

@carlossg done! 👍

DRY-ing up adding Github Token to URLs
carlossg pushed a commit that referenced this pull request Jun 16, 2014
@carlossg
Copy link
Collaborator

Squashed and merged, thanks

@carlossg carlossg closed this Jun 16, 2014
@petems
Copy link
Contributor Author

petems commented Jun 16, 2014

Thanks! 👍

@petems petems deleted the fix_empty_github_api_key branch June 16, 2014 13:39
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