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

Document BearerToken#token. #147

Closed
wants to merge 1 commit into from
Closed

Document BearerToken#token. #147

wants to merge 1 commit into from

Conversation

JuanitoFatas
Copy link
Contributor

Is it okay to document the usage of instance variable like this? If it's not please tell me, I will change and send more doc patches. Thanks!

@ixti
Copy link
Member

ixti commented Jun 23, 2014

I don't think it's a good idea to refer to instance variables. They are not part of public API, thus they can be changed any time. By documenting internal things you're in danger of repeating a code itself :))

And in this particular situation, (my bad) token should be a private method actually :D

@ixti
Copy link
Member

ixti commented Jun 23, 2014

Thanks for your help with documentation, anyway.

@ixti
Copy link
Member

ixti commented Jun 23, 2014

After reviewing that code of bearer token, I must admit that encode option will be deprecated actually. I don't know why I ever added that option at all. JSON Web Token specification (Bearer token) does not have anything related to Base64, so I believe it was a stupid copy-pasta from basic authorization header.

@ixti ixti closed this Jun 23, 2014
@JuanitoFatas JuanitoFatas deleted the docs branch June 23, 2014 19:21
@JuanitoFatas
Copy link
Contributor Author

Nice explanation! Thank you so much and your time work on this.

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.

None yet

2 participants