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

Upgrade Brotli to v0.5.2 #63

Closed
wants to merge 1 commit into from
Closed

Upgrade Brotli to v0.5.2 #63

wants to merge 1 commit into from

Conversation

quittle
Copy link

@quittle quittle commented Oct 11, 2016

This upgrades Brotli to v0.5, which is the latest available branch on GitHub.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@anthrotype
Copy link
Member

well, if anything, you want to checkout the submodule at the latest tagged release:
https://github.com/google/brotli/releases/latest
... which is not that branch you're pointing to, but is the tag 0.5.2, with hash 66c14517cf8afcc1a1649a7833ac789366eb0b51

@anthrotype
Copy link
Member

oh, that's already the case!
sorry, I was misled by the PR title, I should have checked first 😬

@quittle
Copy link
Author

quittle commented Oct 11, 2016

CLA re-check

@googlebot
Copy link

CLAs look good, thanks!

@fred-wang
Copy link
Contributor

Note that @khaledhosny has opened #61

@quittle
Copy link
Author

quittle commented Oct 11, 2016

I saw that but figured that it would be better to upgrade to a release commit instead of HEAD of master.

@fred-wang fred-wang mentioned this pull request Oct 12, 2016
@quittle
Copy link
Author

quittle commented Oct 13, 2016

Is Google interested in taking this PR?

@rsheeter
Copy link
Contributor

We be interested :) Thanks for the PR and sorry for the slow response.

However, could I impose on you to bump brotli to the current latest release (0.5.2)? - the commit in this PR doesn't appear to match any currently listed on https://github.com/google/brotli/releases.

@quittle
Copy link
Author

quittle commented Oct 26, 2016

This commit is actually 0.5.2. they are both tagged to the same commit ID. I can update the commit message if you like.

@quittle quittle changed the title Upgrade Brotli to v0.5 Upgrade Brotli to v0.5.2 Oct 27, 2016
@quittle
Copy link
Author

quittle commented Oct 27, 2016

Okay, I've updated the commit message to reflect the more accurate version number

@quittle quittle closed this Nov 15, 2016
@quittle quittle deleted the brotli-v0.5-upgrade branch November 15, 2016 03:40
@quittle
Copy link
Author

quittle commented Nov 15, 2016

I have created a new PR for this change after adding tests and fixing build issues. I must have missed the conversion of Brotli to use a C interface instead of the C++ one it used when I converted this commit from one used for a personal project. See #65 for the new revision, which is a bit more invasive

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.

5 participants