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.

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

[proposal] Warning banners configuration structure #945

Closed
choldgraf opened this issue Sep 24, 2022 · 5 comments
Closed

[proposal] Warning banners configuration structure #945

choldgraf opened this issue Sep 24, 2022 · 5 comments

Comments

@choldgraf
Copy link
Collaborator

Context

Currently we have a PR implementing warning banners to direct users to the latest version of docs if they're on an old version. However it has stalled out because it wasn't clear the right configuration mechanism to use. This is a proposal for something so we can move forward.

Constraints for configuration

I believe the most important points @bryevdv mentioned were:

  • There should be a single source of truth for the configuration across multiple docs sites (so nothing that embedded in the HTML of each site)
  • We don't want to have to change more than one configuration when a new version is added (to reduce toil and forgetting to update things). So no "package version-specific config with each version".

Proposal

Here's what I'd suggest we do to move forward:

  • Currently we accept a versions.json file that contains a list of configuration entries, each one corresponding to a version.
  • We allow versions.json to optionally be a dictionary instead of a list. If it is a dictionary, we assume it is a collection of key:val pairs to configure the versions behavior.
  • If it is a dictionary, there must be an entry called items, the value for items will be the list of version entries that currently populates versions.json.
  • In addition to items, there may be other configuration to control behavior. We propose two optional values:
    • preferred_version: is a unique ID to point to a version that users should use (e.g. stable). If the user is on a docs build that does not have this version, they'll get the warning banner with a message saying they're on an outdated version.
    • development_version: is a unique ID to point to the "development version" of the docs. The only difference this makes is that this version will display a "this is a development version" message, instead of the "this is an outdated version" message.

Feedback

  • Any objections to this proposal?
  • Anything you think we should change?

References

@choldgraf choldgraf changed the title Warning banners configuration proposal [proposal] Warning banners configuration structure Sep 24, 2022
@choldgraf
Copy link
Collaborator Author

cc @jarrodmillman who I discussed this with yesterday

@drammock
Copy link
Collaborator

Seems reasonable. Another possibility is to keep it as a list-of-dicts, and have users optionally add a preferred=True item to one entry, and similarly an optional latest=True to one entry.

The reason to (maybe?) prefer this approach is that I think it will end up needing fewer / simpler changes to the existing JS that parses the versions.json data. (That is a hunch based on memories of the code, not based on reviewing it today. Nor is it an expression of strong preference; whoever implements it can choose which way seems easier, as far as I'm concerned.)

@bryevdv
Copy link
Contributor

bryevdv commented Sep 26, 2022

Another possibility is to keep it as a list-of-dicts, and have users optionally add a preferred=True item to one entry, and similarly an optional latest=True to one entry.

I'm not a huge fan on this as it easily leaves open the possibility of erroneously adding preferred and latest to more than one dict, and now you need some validation step if you want to make sure you haven't screwed up your JSON file. I'd rather just have one top -level value to update, which does not leave open that possibility, and is also simpler to automate. I'm not sure which is easier to implement but I have strong opinions about which one is more user-friendly.

@12rambau
Copy link
Collaborator

@choldgraf is this solved by #1354 ?

@drammock
Copy link
Collaborator

closed by #1354. I'll migrate @bryevdv's comment to a new issue, because it's something I missed/forgot about when working on #1354 and I think it wouldn't be too hard to support both JSON file structures

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 a pull request may close this issue.

4 participants