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

Add cargo-c to metrics #37

Merged
merged 1 commit into from
Nov 27, 2019
Merged

Add cargo-c to metrics #37

merged 1 commit into from
Nov 27, 2019

Conversation

Luni-4
Copy link
Member

@Luni-4 Luni-4 commented Nov 11, 2019

This PR fixes #28

Sometimes cargo-c works and sometimes it doesn't, I don't know why though. Do you know how to fix that @lu-zero?

Thanks in advance for your review! :)

@lu-zero
Copy link
Member

lu-zero commented Nov 11, 2019 via email

@Luni-4 Luni-4 force-pushed the cargo-c branch 8 times, most recently from 0031de3 to 704b6aa Compare November 18, 2019 15:11
@Luni-4
Copy link
Member Author

Luni-4 commented Nov 18, 2019

@lu-zero

I've used pkg-config and put the cargo-c tests into a different job. The warnings are disappeared, but the usual cargo-c error remains.

@Luni-4 Luni-4 changed the title WIP: Add cargo-c to metrics Add cargo-c to metrics Nov 22, 2019
@Luni-4 Luni-4 requested a review from lu-zero November 22, 2019 09:05
@Luni-4
Copy link
Member Author

Luni-4 commented Nov 25, 2019

I propose to implement the new frame APIs in another PR, so this one can be landed allowing these APIs to be used. What do you think about that @lu-zero?

@Luni-4 Luni-4 force-pushed the cargo-c branch 2 times, most recently from 6e24137 to d785b21 Compare November 27, 2019 11:20
@lu-zero
Copy link
Member

lu-zero commented Nov 27, 2019

I'm fine with that. Fix the rustfmt lints and push :)

@Luni-4
Copy link
Member Author

Luni-4 commented Nov 27, 2019

I'm fine with that.

Thanks for your comprehension! :)

Fix the rustfmt lints and push :)

Unfortunately, I can't :(

To fix the rustfmt lints, I would have to add the pub unsafe extern "C" fn signature, but that breaks cargo-c since it wants, don't know why, the pub unsafe extern fnone to work.

@lu-zero
Copy link
Member

lu-zero commented Nov 27, 2019

This sounds a bug in cbindgen.

@Luni-4 Luni-4 force-pushed the cargo-c branch 2 times, most recently from 638b140 to e39c250 Compare November 27, 2019 14:09
@Luni-4
Copy link
Member Author

Luni-4 commented Nov 27, 2019

@lu-zero

Fixed rustfmt lints, but it seems that cargo-c doesn't run correctly in this PR. I've tried this very same PR in another branch and it works, sigh.

I think I'm going to land this one the same. What do you think?

@lu-zero
Copy link
Member

lu-zero commented Nov 27, 2019

Go for it, we'll refine later.

@Luni-4 Luni-4 merged commit 6f6ac6a into rust-av:master Nov 27, 2019
@Luni-4 Luni-4 deleted the cargo-c branch November 27, 2019 15:45
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.

Apply cargo-c on av-metrics
2 participants