search

ueberauth / guardian

Elixir Authentication
MIT License
3.44k stars 381 forks source link

Improve documentation for verify_header plug module #441

Closed edouardmenayde closed 6 years ago

edouardmenayde commented 6 years ago

I'm new to elixir and its ecosystem thus i'm starting to learn its ins and outs including guardian. I love how easy testing is with elixir but I couldn't understand why my test couldn't pass thus I had to dig through the code to understand how the header is parsed by default. I think adding a few line of documentation could help the next guy experience.

oskar1233 commented 6 years ago

IMHO the stuff you covered is more about understanding building HTTP request with authorization header than about verify_header module itself. Maybe readme or even some separate guide would be better place to write about it?

edouardmenayde commented 6 years ago

@oskar1233 I agree it might be best in the readme. Do I create a new PR or do I amend the existing commit ?

doomspork commented 6 years ago

@edouardmenayde thanks for getting involved! I agree with @edouardmenayde that this doesn't really belong in the code documentation, especially not without more context.

Perhaps we should look into adding a section to the README for now on testing best practices?

Thoughts @ueberauth/developers?

oskar1233 commented 6 years ago

Maybe some subheading in the basics section? Just because that's what you usually do in the beginning and boosts your API understanding :)

edouardmenayde commented 6 years ago

Hi! It's been some days since the last comment thus I think if there's any strong insight against adding that to the readme It would have been raised by now. I took the initiative to amend my last commit and move the documentation to the readme.

edouardmenayde commented 6 years ago

@yordis I fixed the typo and gave some more context to the test. I tried giving much more context with the controller etc. but It becomes quickly more of a blog post than a little hint to test guardian.

Moggers commented 6 years ago

This just saved me from hours of headache. I hadn't used the Authorization header before and didn't realize that it had to look like authorization: <type>: <token>. Was digging through the suggested documentation and saw an example with a put_req_header call

edouardmenayde commented 6 years ago

@Moggers really happy my headache saved you from having one :)

edouardmenayde commented 6 years ago

@doomspork I fixed my mistake, this should be ok to merge :-)

doomspork commented 6 years ago

Thank you @edouardmenayde! 👍