Introduce a GitHub Actions workflow for publishing a release and fix npm publish warnings about the package.json format
#18350
Conversation
This file contains 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 commit migrates this functionality away from the bots. Note that the NPM token must be configured as a repository secret before this workflow can execute. The following resources are relevant for this patch: - Publishing packages to the NPM registry: https://docs.github.com/en/actions/publishing-packages/publishing-nodejs-packages#publishing-packages-to-the-npm-registry - Creating secrets for a repository: https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions#creating-secrets-for-a-repository
This commit removes the following warnings from the `npm publish` output: ``` npm warn publish npm auto-corrected some errors in your package.json when publishing. Please run "npm pkg fix" to address these errors. npm warn publish errors corrected: npm warn publish Removed invalid "scripts" npm warn publish "repository.url" was normalized to "git+https://github.com/mozilla/pdfjs-dist.git" ``` For the "scripts" section it turns out that if the package doesn't have any scripts it's expected to explicitly set it to an empty object; refer to npm/cli#6918 and denoland/dnt#414.
wojtekmaj
reviewed
Jul 1, 2024
This was referenced Jul 1, 2024
timvandermeij
added a commit
to timvandermeij/pdf.js
that referenced
this pull request
Jul 13, 2024
The Git logic for pushing to the `pdfjs-dist` repository was already removed in PR mozilla#18350, but it was forgotten to also remove the logic to create a Git repository in the first place. This commit fixes the open action for that from mozilla#18358 (comment). Moreover, we implement the suggestion from mozilla#18358 (comment) about moving the Git URL to a separate variable for consistency and we update the homepage URL to the HTTPS scheme to avoid an HTTP -> HTTPS redirect and enforce TLS-encrypted connections.
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.
The commit messages contain more details about the individual changes.
Notes:
on_release.jsfile botio-files-pdfjs#45 being merged and deployed to the bots first (to avoid publishing twice).NPM_TOKENrepository secret being configured in this repository, with the same token that the bots currently have.on: pull_requesttrigger so that I could trigger the workflow from my branch (because releases require it to be merged tomasterfirst which is inconvenient for testing) and adding the--dry-runoption to thenpm publishcommand so that we can see what e.g. the release tarball would contain without actually having to publish anything yet. You can find the logs of the dry-run for the first commit at https://github.com/timvandermeij/pdf.js/actions/runs/9732441891/job/26858099909?pr=11 and with the second commit applied at https://github.com/timvandermeij/pdf.js/actions/runs/9732541964/job/26858344999?pr=11 (showing that the warnings are now gone). Release tarball correctness can be verified by comparing the file listing in the logs to the file listing on https://www.npmjs.com/package/pdfjs-dist/v/latest?activeTab=code, and seeing that the "total files" property of 334 and the "unpacked size" property of 36 MB matches what's listed on NPM for the current release.pdfjs-distrepository has been broken for a long time and the current logic fails to run on GitHub Actions. To keep the scope of this patch small and to only port the logic we actually need and have functional right now I'll make a follow-up issue for revamping the Git publishing part in a separate scope (because that'll have to be re-implemented anyway).Fixes a part of #11851.