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

C and OCaml bindings for Tapir #66

Merged
merged 11 commits into from
May 26, 2018
Merged

C and OCaml bindings for Tapir #66

merged 11 commits into from
May 26, 2018

Conversation

Willtor
Copy link
Contributor

@Willtor Willtor commented May 21, 2018

When you wish upon a star, makes no difference who you are. Because in space, no one can hear you scream.

@neboat
Copy link
Collaborator

neboat commented May 22, 2018

I'm excited to see these additions. The changes looks good to me. Do you have a test case handy for verifying that these changes work?

@Willtor
Copy link
Contributor Author

Willtor commented May 22, 2018

I don't have anything small that tests them. DEF will use these changes as soon as they're ready. I can commit and push those changes, though I was planning on doing it in the opposite order. Is that what we need?

@Willtor
Copy link
Contributor Author

Willtor commented May 22, 2018

Okay! I've created a tapir-transition branch to DEF:
https://github.com/Willtor/def/tree/tapir-transition
And I added the c28866e changeset that works with that branch (and fixes a missing macro bug).

@wsmoses wsmoses self-requested a review May 23, 2018 13:36
wsmoses
wsmoses previously approved these changes May 23, 2018
Copy link
Owner

@wsmoses wsmoses left a comment

Choose a reason for hiding this comment

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

LGTM, though yeah at one point some unit tests would be nice.

@wsmoses
Copy link
Owner

wsmoses commented May 23, 2018

Sorry, apparently CI wasn't configured properly for PR's from external forks, if you don't mind repushing to trigger a build (and then you'll be able to merge once the build passes).

@Willtor
Copy link
Contributor Author

Willtor commented May 23, 2018

I'll add the OCaml stuff into the unit tests for it. That'll cover the C bindings, too, since OCaml uses them. When I push that change, will that trigger the build?

@Willtor
Copy link
Contributor Author

Willtor commented May 23, 2018

Like a boss!

@neboat neboat merged commit 93adcc4 into wsmoses:master May 26, 2018
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.

None yet

3 participants