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.

Sign up for GitHub

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

Update Blacklist settings docs #308

Merged
merged 2 commits into from
Sep 25, 2020
Merged

Update Blacklist settings docs #308

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a23f954
Update Blacklist settings docs
saadmk11 Sep 25, 2020
e684d0e
Update settings.rst
saadmk11 Sep 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/settings.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ tokens submitted to the refresh view will be added to the blacklist.
When set to ``True``, causes refresh tokens submitted to the
``TokenRefreshView`` to be added to the blacklist if the blacklist app is in
use and the ``ROTATE_REFRESH_TOKENS`` setting is set to ``True``.
You need to add ``'rest_framework_simplejwt.token_blacklist',`` to your
``INSTALLED_APPS`` in the settings file to use this settings.
Copy link
Member

Choose a reason for hiding this comment

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

There's a little typo here. It should be "...in the settings file to use this setting." In your version, the word "settings" at the end of the sentence is incorrectly pluralized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davesque 👍 #310


Learn more about :doc:`/blacklist_app`.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be a :ref:, not a link? (Not good with docs). Otherwise, I think it's ok to add this. Just capitalize "Django" or remove the word entirely to be in accordance with DSF.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just capitalize "Django" or remove the word entirely to be in accordance with DSF.

Sure, I think removing it would be better. So, removed it. :)

It looks like this. ref: https://www.sphinx-doc.org/en/master/usage/restructuredtext/roles.html#role-doc
Screenshot from 2020-09-25 21-29-49

Maybe enabling Read the Docs pull request builder would be great for reviewing docs.:)

Copy link
Member

Choose a reason for hiding this comment

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

Hey @saadmk11 Never heard of it, but if you'd like to help incorporate that to, I'd love to see it! Would make my life easier :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Andrew-Chen-Wang Sure! 👍

It's fairly simple you just need to enable it from Read the Docs dashboard.
You need to go to Admin > Advanced settings for this project on Read the Docs dashboard and Enable the Build pull requests for this project option.

More details here: https://docs.readthedocs.io/en/stable/guides/autobuild-docs-for-pull-requests.html

Would love to contribute to this project, I'm new to this codebase but I'll look through some issues. :)

Copy link
Member

Choose a reason for hiding this comment

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

Awesome! I may be a maintainer, but I don't have access to bumping versions, PyPi, and RTD so if there is anything that we do need to enable externally from GitHub, please let me know as only David has those aforementioned capabilities!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, you need to enable the option that I mentioned from the RTD dashboard.

Copy link
Member

Choose a reason for hiding this comment

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

Again, I don't have that capability. It was just a future reference in that you'll need to tell me detailed of what to enable.


``UPDATE_LAST_LOGIN``
----------------------------
Expand Down