Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Docs for feature flag #5043
Docs for feature flag #5043
Changes from 4 commits
3fbe47d
4adef9e
78389bb
c7c7370
534026d
9bbd946
2f8cedb
3e83667
819e5c8
5d5c2ff
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could use a rst role to keep the list updated, something like #4451 (comment) https://github.com/rtfd/readthedocs.org/pull/4451/files#diff-d4de2c39f7f56cdad5a95921ae1398ec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stsewd
I didn't get it.
Do you want to render the whole list (with description) with rst role? or just the description? or any other thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List and description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stsewd
I have tried it and i suppose that the function for "role" is working fine. However I am not able to get the output at the docs.
Can you please help me in this. I am unable to find the mistakes.
The output I'm getting is
featureflags:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stsewd
It was a wonderful idea. I managed to implement it successfully and now the flags with their description are rendering as desired automatically.
Here is the image of the feature flags docs page - image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love adding features like this to our docs! 💯
I would suggest a change here: instead of using a role to put all of the different feature flags in a single block, this should mimic what the django setting role does.
Also, if you were to implement a feature like this normally, you'd use a
directive
, not arole
. I do thinkrole
would be preferred here if you were to match how we are using the django settingrole
. It allows us to put more commentary around the output.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@agjohnson
I am a little confused.
This is what I'm getting now.
nodes.Text
.But doing this, we will lost the automatic rendering of the docs.
Also, the docs will look like this - image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to fully automate things, there might be some feature flags we don't want documented or don't need documented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the explanation.
I have followed your advice and changed the pattern of the role similar to
djangosettings role
.