-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Conversation
I request to please suggest imorovements for english. |
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.
Small language changes.
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.
Please follow our guidelines for contributing to documentation https://docs.readthedocs.io/en/latest/docs.html
@stsewd |
.. _github: https://github.com/rtfd/readthedocs.org | ||
|
||
Available Flags | ||
--------------- |
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.
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 a role
. I do think role
would be preferred here if you were to match how we are using the django setting role
. 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.
- In the rst file, the use of the role should be like this.
which will render the feature flags with its description as
``FEATURE_1``: :featureflags:`FEATURE_1` ``FEATURE_2``: :featureflags:`FEATURE_2` ``FEATURE_3``: :featureflags:`FEATURE_3`
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
.
Can you upload a screenshot so we can see how it will looks like? Thanks! |
@humitos But the look of it may change as per the comment by @agjohnson above. |
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.
Looks good. 👍
Fixes #5041