-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fix previous releases sidebar feature for ecosystem projects #267
Conversation
### Summary The previous releases section on the documentation sidebar includes the current version (0.5), which is unnecessary. The link is also broken right now (will be fixed in Qiskit/qiskit_sphinx_theme#267), which is another motivation to remove this from the current docs. This PR changes the list so the current version won't be included.
### Summary The previous releases section on the documentation sidebar includes the current version (0.5), which is unnecessary. The link is also broken right now (will be fixed in Qiskit/qiskit_sphinx_theme#267), which is another motivation to remove this from the current docs. This PR changes the list so the current version won't be included. (cherry picked from commit d12fb7b)
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 the javascript in this PR should be extracted out into it's own module for re-usability in future. We could have a general utils.js
file where we keep little js functions like this that we might want to use for other things and also do some proper automated testing on them
Prework for #208. Abby and I decided that we want Jest tests for two purposes: 1. Checking that web components are in sync between the Pytorch and Furo themes, e.g. we didn't forget to update one of the folders. 1. We could do this either by using Snapshot testing or by simply checking that the file contents are equal. 2. We have some non-trivial logic in #267 and want to write automated unit tests. This PR adds the basic infrastructure, like setting up CI to install Node.js. That will make it easier to review follow-up PRs that add actual tests. > We could do this either by using snapshot testing I think we'd benefit from adding snapshot testing for the top nav bar rendering correctly. I want to do this in a follow up PR so we can properly decide if we want to add snapshots or not; the decision to use snapshots is separate from whether to use Jest generally.
…into previous-versions-fix
…into previous-versions-fix
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.
Copy and paste of the Furo file. The Jest test is set up to ensure that the tests pass for both Furo and Pytorch.
We aren't testing that the file contents are identical between Furo and Pytorch because we may want utils.js
to diverge. For example, maybe we add a util to Furo that is not needed for Pytorch.
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.
🚀
Closes #235. As summarized in #235 (comment), we were only handling Terra properly.
I used test driven development to ensure I handled all the edge cases.
Leverages ecosystem redirects
We are now successfully redirecting ecosystem projects, e.g.
documentation/experiments/
toecosystem/experiments
. That simplifies the edge cases we need to handle: we only need to handle Terra (documentation/
) and Ecosystem (ecosystem/<project>
).Adds
utils.js
fileHaving a dedicated JavaScript file for our logic will make it easier to find all our custom logic. It also allows us to test the contents with Jest.