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

Fix previous releases sidebar feature for ecosystem projects #267

Merged
merged 10 commits into from
May 4, 2023

Conversation

Eric-Arellano
Copy link
Collaborator

@Eric-Arellano Eric-Arellano commented Apr 12, 2023

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/ to ecosystem/experiments. That simplifies the edge cases we need to handle: we only need to handle Terra (documentation/) and Ecosystem (ecosystem/<project>).

Adds utils.js file

Having 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.

Eric-Arellano

This comment was marked as outdated.

github-merge-queue bot pushed a commit to qiskit-community/qiskit-experiments that referenced this pull request Apr 18, 2023
### 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.
mergify bot pushed a commit to qiskit-community/qiskit-experiments that referenced this pull request Apr 18, 2023
### 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)
Copy link
Collaborator

@javabster javabster left a 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

qiskit_sphinx_theme/pytorch_base/sidebar.html Outdated Show resolved Hide resolved
@Eric-Arellano Eric-Arellano changed the title Fix previous releases sidebar feature for ecosystem projects [blocked] Fix previous releases sidebar feature for ecosystem projects Apr 26, 2023
@Eric-Arellano Eric-Arellano changed the title [blocked] Fix previous releases sidebar feature for ecosystem projects [blocked by #282] Fix previous releases sidebar feature for ecosystem projects Apr 26, 2023
@Eric-Arellano Eric-Arellano marked this pull request as draft April 26, 2023 14:24
Eric-Arellano added a commit that referenced this pull request Apr 27, 2023
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.
@Eric-Arellano Eric-Arellano changed the title [blocked by #282] Fix previous releases sidebar feature for ecosystem projects Fix previous releases sidebar feature for ecosystem projects Apr 27, 2023
@Eric-Arellano Eric-Arellano marked this pull request as ready for review April 27, 2023 15:52
Copy link
Collaborator Author

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.

Copy link
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

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 this pull request may close these issues.

Previous Releases link broken for ecosystem projects
3 participants