Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Cross bindings #77

Merged
merged 10 commits into from
Oct 10, 2020
Merged

Conversation

YruamaLairba
Copy link
Contributor

It add experimental bindings for arm and aarch64
Also update and improve the systool README

@YruamaLairba
Copy link
Contributor Author

@Janonard i can add bindings for *-pc-windows-gnu targets but i don't know if it worth. Even if it's not identical to x86_64-pc-windows-msvc, it's fully compatible. The only noticeable difference is the signed/unsigned bindings on enum, which is irrelevant.

@JOOpdenhoevel JOOpdenhoevel self-requested a review July 19, 2020 07:31
@JOOpdenhoevel JOOpdenhoevel self-assigned this Jul 19, 2020
@JOOpdenhoevel JOOpdenhoevel added 📖 Documentation An effort to improve the documentation ✨ Feature New feature or request labels Jul 19, 2020
@JOOpdenhoevel JOOpdenhoevel added this to the v0.6 milestone Jul 19, 2020
@JOOpdenhoevel
Copy link
Contributor

I guess you can merge the gnu and msvc bindings. I used to be a little anxious about mixing bindings that aren't 100% identical, but given that the difference between signed and unsigned bindings is never relevant (and would even be annoying when you want to target both targets), I now think it's okay.

Also, does the existence of this PR mean that you're abandoning #64?

@YruamaLairba
Copy link
Contributor Author

Also, does the existence of this PR mean that you're abandoning #64?

Yes. If i remember correctly, you forked it, did some change, and merged back to develop branch

@YruamaLairba
Copy link
Contributor Author

@Janonard i updated the supported target section of README.md. I tried to separate binding concept from supported target concept. I'm not sure if my explanation are clear between supported target, usable target, experimental feature gated by experimental-targets

@JOOpdenhoevel
Copy link
Contributor

I think you mean the right thing, but it isn't that clear yet: A target has three states: It may be missing from the sys-crate, it may be included but not supported, or it may be included and supported. The key difference is that for a supported target, there has to be maintainer who can trace problems regarding that target by executing a plugin with a certain host of that target. This has to be stated clearly and precisely.

@YruamaLairba
Copy link
Contributor Author

I fixed explanation about target support requirement. For the rest, i don't know how to make it better. Maybe by presenting tables differently ? One difficulty in explanations is the "experimental-targets" feature, it doesn't really act as a gate for "unsupported but usable" targets. It may more clear if we rename this feature as "experimental-bindings", isn't it ?

@JOOpdenhoevel
Copy link
Contributor

#80 is this better? If so, please pull develop and add your ARM bindings to Tier 2, so I can merge this PR!

@YruamaLairba YruamaLairba marked this pull request as ready for review October 9, 2020 22:34
@YruamaLairba
Copy link
Contributor Author

i hope i fixed all my errors. Some of them was not detected by the automatic check

Copy link
Contributor

@JOOpdenhoevel JOOpdenhoevel 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!

@JOOpdenhoevel JOOpdenhoevel merged commit 5d18c09 into RustAudio:develop Oct 10, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
📖 Documentation An effort to improve the documentation ✨ Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants