Update PDF.js to v2.14.137 and improve automation/documentation of process #720
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This standardizes the commit messages when PDF.js is updated and makes sure that the version number is included. To do this it was necessary to build the template as part of the `bin/update-pdfjs` script rather than do that afterwards. The instructions for `bin/update-pdfjs` have also been updated to make it clear that the script needs to be invoked via `make update-pdfjs` rather than run directly, as is done in other repositories.
Update PDF.js using bin/update-pdfjs.
robertknight
changed the title
This was referenced Apr 6, 2022
lyzadanger
reviewed
Apr 7, 2022
Comment on lines
+12
to
+14
| # 1. Create a new branch and run `make update-pdfjs`. This will download the | ||
| # newest release of PDF.js, update the HTML template for the viewer and | ||
| # commit the changes. |
There was a problem hiding this comment.
I'm a little concerned that this script looks so similar to that in other projects that devs might "read past" this different instruction. What "happens" if this script is run directly instead of with make update-pdfjs. Is it possible to build in a safety check?
There was a problem hiding this comment.
Yes. I have pushed a commit that adds a check, using the presence of the TOX_ENV_NAME var to test if the script is being run directly or via tox.
robertknight
force-pushed
the
update-pdfjs-apr-2022
branch
from
2248566 to
6c66502
Compare
lyzadanger
approved these changes
Apr 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update PDF.js to v2.14.137, matching hypothesis/browser-extension#799 (almost, this update is a few minor updates newer).
The
bin/update-pdfjsscript has also been updated to improve the documentation on how to run it and auto-commit the results. In particular the comments in the script now note that it should be run viamake update-pdfjsrather than being run directly as a user would do in other repositories. This is necessary to set up the Python environment for thebin/create_pdf_template.pyscript. Auto-committing the result ensures that the PDF.js version number is included in the commit message and the correct directories are committed. A similar change was recently made for the browser extension in hypothesis/browser-extension#799.