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

Remove obsolete pdfjs-dist code from the Gulpfile #18434

Merged
merged 1 commit into from
Jul 13, 2024

Conversation

timvandermeij
Copy link
Contributor

@timvandermeij timvandermeij commented Jul 13, 2024

The Git logic for pushing to the pdfjs-dist repository was already removed in PR #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
#18358 (comment).

Moreover, we implement the suggestion from #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.

Fixes #18357. Once this is approved and merged, we can archive the pdfjs-dist repository.

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.
@timvandermeij
Copy link
Contributor Author

I have diffed the contents of the old and new build/dist directory and confirmed that in the new version the .git directory is gone and no other relevant changes to files were made (the only changes were due to the Git hash, and therefore the build number, changing between the branches).

I have also diffed the contents of the NPM publish command, in dry-run mode, that we use the publish release workflow. I used the following set of commands:

$ npm publish ./build-old/dist --provenance --dry-run | tee old.log
$ npm publish ./build-new/dist --provenance --dry-run | tee new.log
$ diff old.log new.log 
1c1
< + [email protected]
---
> + [email protected]

Note that also here the only change is the build number due to the extra commit on the new branch, but more importantly the list of files that are part of the release tarball has no changes.

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

r=me, thank you!

@timvandermeij timvandermeij merged commit 5059187 into mozilla:master Jul 13, 2024
9 checks passed
@timvandermeij timvandermeij deleted the pdfjs-dist branch July 13, 2024 18:28
@timvandermeij timvandermeij removed the request for review from calixteman July 13, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement publishing a release to the pdfjs-dist GitHub repository
2 participants