8000 Fix binding-functions when building out of source by isaacbrodsky · Pull Request #188 · uber/h3 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix binding-functions when building out of source #188

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

Merged

Conversation

isaacbrodsky
Copy link
Collaborator

The binding functions scripts were looking in the original source directory, rather than for the generated h3api.h which is now under the binary directory. Also updated binding_functions.sh to fail the build when the file it expects is not found.

@coveralls
Copy link
coveralls commented Feb 13, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling fb8cf44 on isaacbrodsky:binding-functions-out-of-source into 5a55394 on uber:master.

@nrabinowitz
Copy link
Collaborator

This looks good, but can we add a Travis test that verifies binding_functions can be built correctly?

Copy link
Collaborator
@nrabinowitz nrabinowitz left a comment

Choose a reason for hiding this comment

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

Looks good once the CI test is set up and green - thanks!

appveyor.yml Outdated
@@ -39,7 +39,10 @@ environment:
build_script:
- cmake "-G%GENERATOR%" -H. -Bbuild
- cmake --build build --config "%CONFIG%"
# Intentionally failing the build to check the test
#- cmake --build build --config "%CONFIG%" binding-functions
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this still be here? Does leaving it commented out imply that the test should fail but doesn't?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The test should fail, but it just emits an error message and marks it as succeeded anyways. I'd like to take a look at that tomorrow and see if I can get it working before merging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated the PR so Appveyor also tests the binding-functions target.

@isaacbrodsky isaacbrodsky merged commit 67b4558 into uber:master Feb 15, 2019
@isaacbrodsky isaacbrodsky deleted the binding-functions-out-of-source branch February 15, 2019 19:09
mrdvt92 pushed a commit to mrdvt92/h3 that referenced this pull request Jun 19, 2022
…f-source

Fix binding-functions when building out of source
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.

4 participants
0