-
Notifications
You must be signed in to change notification settings - Fork 329
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
many users struggle with version switcher/banner handling #1629
Comments
Clarifications will go a long way to help with this feature. I've been banging my head against it for about a day. I started digging into the JS and realized that the version switcher expects semver compliant versions. My project doesn't always use semver compliant names, which was making the whole thing act funny. |
I have a comment and maybe a suggestion on this topic. Indeed looking at the sphinx documentation, using I assume that many people, like initially myself, believe that Adidtional question, does the |
glad you agree :)
The meaning of those variables is determined by Sphinx (not by theme authors), and is described in their docs. Briefly:
I am struggling to follow exactly what the suggested change is here. Is it a change to the guidance in our docs? Or a change to the key names we use in
No it does not. It's a neat idea, but not super easy, because which version is
it's probably also possible to do something similar in the theme's |
I believe this is exactly the point, where the confusion of many users starts. What should be called a
I understand this, but as mentioned above
Here I intended to suggest to change the key name in the JSON file to "stable".
As you see I am not so much in the details of writing themes and this indeed seems more complicated than anticipated. So for now ite seems easier to manually edit the |
Correct me if I'm wrong, but I think this means that the most we could do here is "make our own docs a bit clearer". I'm happy to try to do that / review other's PRs that do so. We do already link to the Sphinx definitions in the switcher-version-match section but maybe we should also do so in the warning banner section?
Ah, point taken --- I had lost track of the fact that this was about the warning banner too and not just the version switcher. So I guess the warning banner section needs to make clear that it is the
I'm not sure I agree with this proposal. "preferred" is more generic, which I think is more accomodating / less confusing for certain cases. In particular, projects that maintain multiple "stable" versions (think of the era of python 2.x and 3.x both being supported, for example), or cases where the developers want users to always be reading the "dev" version of the docs (I've encountered this at least once, but I forget where). I think I can envision now some concrete improvements to the docs. A couple are fairly straightforward and mentioned above:
Additionally, I think it's probably worthwhile to add some background/explanation about why it works this way. Namely:
That last bullet reveals another non-docs thing we could do that might (?) make things easier: change the JSON field name from |
Indeed I did not think about cases with multiple stable versions. So, I do agree that improoving the documentation is the best thing to do. As you said, once the users understands how the existing scheme works, he should be able to work with what is there. In case really needed, one can still define additional varibales in |
Explaining in the documentation that we have to define both the version and release variables would be helpful. Sphinx doesn't list either of those fields as required. You can remove any of the Project Configuration variables from conf.py and still get a "successful" build. It's an example of variable defaults at its best and worst. I don't need to ever set the values, but if downstream tools depend on them, I'll get behavior that I don't expect. PyData Sphinx Theme introduces functionality tied to variables that, when initialized to their defaults, causes unexpected behavior. Maybe a two prong approach? Update the documentation and add a utility function that validates version and release that issues a warning if they're not set? I'm not super familiar with the inner workings of the Sphinx build process, so I don't know how difficult the latter might be. The documentation update could be as simple as stating that both those variables are required fields in the conf.py for compatibility with this theme. |
we use the
release
value and not theversion
value fromconf.py
to decide whether to show a version warning banner. We are assuming folks follow what is stated in the Sphinx docs forrelease
:the fact that folks keep stumbling around the version switcher and the warning banner suggests that either (1) what we're doing is too fragile/magic, or (2) we're not documenting it clearly enough.
Originally posted by @drammock in #1445 (comment)
The text was updated successfully, but these errors were encountered: