Skip to content

Conversation

@illia-v
Copy link
Contributor

@illia-v illia-v commented Dec 26, 2018

No description provided.

@illia-v illia-v changed the title Update docs according to c5f32c337d8d191e93ab80152640090ddb664639 Update docs according to #215 Dec 26, 2018
@coveralls
Copy link

coveralls commented Dec 26, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling ae2dad8 on illia-v:more-allowed-data-structures-docs into 03d76f5 on vimalloc:master.

@vimalloc
Copy link
Owner

vimalloc commented Jan 2, 2019

I'm not actually sure I want to update this. I think ('headers',) could be a lot more confusing then ['headers'] for people who are not as familiar with python, and tuples or lists should work fine for this setting. Thoughts?

@illia-v
Copy link
Contributor Author

illia-v commented Jan 3, 2019

Agree, but JWT_TOKEN_LOCATION does default to ('headers',).
We can change it to default to just a string and then change the docs. Will it be good?

@vimalloc
Copy link
Owner

vimalloc commented Jan 9, 2019

It does default to ('headers',), but functionally if it defaults to that or ['headers'] doesn't have any real affect for users, and the later is (imo) generally better understood. I don't really want to change it to just headers either, as having it displayed as a list/sequence/etc in the documentation makes it easier to see at a glance that you can have multiple token locations instead of just one, without having to read the fine print as it were.

These are my thoughts on the matter, but I would love to hear what you think about that. 👍

@vimalloc
Copy link
Owner

vimalloc commented Jan 9, 2019

Should also note I do want to update the documentation to say that you can use a sequence, set, etc instead of just a list for these fields. I think there is absolutely value in that 👍

@illia-v illia-v force-pushed the more-allowed-data-structures-docs branch from 583628e to ae2dad8 Compare January 10, 2019 20:02
@illia-v
Copy link
Contributor Author

illia-v commented Jan 10, 2019

No problem in changing it to list :)

@vimalloc vimalloc merged commit b4ab25a into vimalloc:master Jan 10, 2019
@vimalloc
Copy link
Owner

Right on. Thanks again for contributing! 👍

@illia-v illia-v deleted the more-allowed-data-structures-docs branch January 11, 2019 17:41
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