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

Updated h3 core to 3.4.0 and updated tests #13

Merged
merged 2 commits into from
Mar 14, 2019
Merged

Conversation

squiidz
Copy link
Contributor

@squiidz squiidz commented Feb 17, 2019

Fixed the issue #7. This pull request doesn't add the newest features from the core library it only update the ones already provided in the bindings

@CLAassistant
Copy link

CLAassistant commented Feb 17, 2019

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

coveralls commented Feb 17, 2019

Pull Request Test Coverage Report for Build 31

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 19: 0.0%
Covered Lines: 202
Relevant Lines: 202

💛 - Coveralls

@bsr203
Copy link

bsr203 commented Feb 18, 2019

@squiidz thanks for the update. quickly scrolling through, I only see one go test file is modified. Have you only had to run update-h3.sh. I wonder is that easy for a future update if I too only use a subset of the API and core didn't have any breaking API change.

@squiidz
Copy link
Contributor Author

squiidz commented Feb 18, 2019

@bsr203 yes, I changed the version in H3_VERSION and used update-h3.sh and updated the test file.

@nrabinowitz nrabinowitz requested a review from jogly February 19, 2019 17:23
@nrabinowitz
Copy link
Collaborator

Thanks for your contribution. Please note that @joegilley, the primary maintainer of h3-go, is out and won't be able to respond to this for several more weeks. Please hold tight until then - thanks!

Copy link
Collaborator

@jogly jogly left a comment

Choose a reason for hiding this comment

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

two very minor comments for cleanup. initially, I was hesitant to update the core version without parity but I don't see any harm in making this library better for the portion of the API it does support while not compromising what it doesn't (yet). well done!

please don't forget to accept the CLA, I can't merge this without it :)

h3_test.go Outdated Show resolved Hide resolved
h3_test.go Outdated Show resolved Hide resolved
@squiidz
Copy link
Contributor Author

squiidz commented Feb 28, 2019

@joegilley Cleanup done :)

@squiidz
Copy link
Contributor Author

squiidz commented Mar 5, 2019

@joegilley up

@jogly jogly merged commit 84b7920 into uber:master Mar 14, 2019
@jogly jogly mentioned this pull request Jun 3, 2019
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.

7 participants