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

Use consistent path for downloadable satre.xlsx spreadsheet #295

Merged
merged 6 commits into from
Oct 11, 2023

Conversation

manics
Copy link
Member

@manics manics commented Oct 10, 2023

✅ Checklist

  • This pull request has a meaningful title.
  • If your changes are not yet ready to merge, you have marked this pull request as a draft pull request.

☑️ Maintainers' checklist

  • This pull request has had the appropriate labels assigned
  • This pull request has been added to the SATRE backlog project board
  • This pull request has been assigned to one or more maintainers

⤴️ Summary

Currently we use the Sphinx :download: role to make the satre.xlsx spreadsheet available, but this results in a random hash being included in the published path. This is annoying since we can't link directly to the spreadsheet.

This PR is a workaround so that satre.xlsx is always in the base directory, without a prefix, so in future we can use
https://satre-specification.readthedocs.io/en/stable/satre.xlsx
instead of
https://satre-specification.readthedocs.io/en/stable/_downloads/<random-hash-which-changes-every-release>/satre.xlsx

🌂 Related issues

🙋 Acknowledging contributors

@manics manics added the bug Something isn't working label Oct 10, 2023
@manics
Copy link
Member Author

manics commented Oct 10, 2023

The linkcheck step is still failing.... without indicating what the error is 🙁. It runs fine locally.

@manics
Copy link
Member Author

manics commented Oct 10, 2023

Another benefit of a consistent URL is we could automatically compare the last stable (tagged) satre.xlsx with the version from a PR, and use https://github.com/simonw/csv-diff to describe the differences that need to be made to an existing evaluation.

@manics manics force-pushed the spreadsheet-fixed-path branch 2 times, most recently from 0832e0d to eed30d7 Compare October 10, 2023 22:02
@manics manics requested a review from a team October 10, 2023 22:04
Copy link
Contributor

@craddm craddm left a comment

Choose a reason for hiding this comment

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

LGTM

@manics manics merged commit 3f782c1 into sa-tre:main Oct 11, 2023
4 checks passed
@manics manics deleted the spreadsheet-fixed-path branch October 11, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants