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 Shaka Player version numbers #11396

Closed
wants to merge 1 commit into from
Closed

Fix Shaka Player version numbers #11396

wants to merge 1 commit into from

Conversation

joeyparrish
Copy link
Contributor

This corrects the release numbers in Shaka Player v2.0.3, v2.0.4, and
v2.1.3.

Closes shaka-project/shaka-player#869

Essential checklist

  • I'm the author of this library
    • I would like to add link to the page of this library on CDNJS on website / readme
  • This lib was not found on cdnjs repo
  • No already exist / duplicated issue and PR
  • The lib has notable popularity
    • More than 100 [Stars / Watchers / Forks] on [GitHub / Bitbucket]
    • More than 500 downloads stats per month on npm registry
  • Project has public repository on famous online hosting platform (or been hosted on npm)

Git commit checklist

  • The first line of commit message is less then 50 chars, be clean and clear, easy to understand.
  • The parent of the commit(s) in the PR is not old than 3 days.
  • Pull request is sending from a non-master branch with meaningful name.
  • Separate unrelated changes into different commits.
  • Use rebase to squash/fixup dummy/unnecessary commits into only one commit.
  • Close corresponding issue in commit message (can't, the issue is in another repo)
  • Mention related issue(s), people in commit message, comment.

This corrects the release numbers in Shaka Player v2.0.3, v2.0.4, and
v2.1.3.

Issue shaka-project/shaka-player#869
Copy link
Contributor

@PeterBot PeterBot left a comment

Choose a reason for hiding this comment

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

@joeyparrish congratulations! 0291219 CI test passed! ✅
Please wait for the further review from the maintainers!

For the details 📃, please take a look at ➡️ https://ci.cdnjs.com/cdnjs/cdnjs/9111, thank you 😀

@PeterDaveHello
Copy link
Contributor

@joeyparrish I'm afraid that this can't be done as we enabled SRI already and this change would make the current users' website broken, if it's only about version number, maybe we don't really need to touch it?

@joeyparrish
Copy link
Contributor Author

I'm not familiar with SRI. What would break if we merged this PR?

@PeterDaveHello
Copy link
Contributor

@joeyparrish Just think SRI is kind of checksum, the modified files would have different hashes so that the old would not work.

Ref: https://www.w3.org/TR/SRI, https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity

@joeyparrish
Copy link
Contributor Author

I see. So based on that MDN article, if we change these files that have already gone out, they may appear to be malicious changes. Do I understand you correctly?

@PeterDaveHello
Copy link
Contributor

@joeyparrish yes, that's why we shouldn't touch the published files.

@joeyparrish
Copy link
Contributor Author

Okay, understood. I'll close this now. Thank you for your feedback.

@PeterDaveHello
Copy link
Contributor

@joeyparrish Thanks.

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.

Wrong version number in released builds
3 participants