Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 a relatively simple way to compile and generate Swift bindings. #1647
base: main
Are you sure you want to change the base?
Add a relatively simple way to compile and generate Swift bindings. #1647
Changes from 2 commits
7fa2515
d080333
9f7fa5b
b28fe4b
6abdfca
baf1810
4761d6c
062e7db
7ceca5a
4c87075
e7b95c6
8e6f413
7386c6e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to make it a code block (easier to copy and more obvious)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your suggestion, I am reviewing the relevant changes. I have found some issues with the description here.
staticlib
andcdylib
correspond to*.a
and*.dylib
libraries respectively. Thestaticlib
in thecrate-type
section does not match the describedcdylib
on documentation, so it needs to be modified.Details:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woops, yes. That should of course be the same in text and code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't ship this CLI anymore. See https://mozilla.github.io/uniffi-rs/tutorial/foreign_language_bindings.html
So we need to rephrase that in a way to use a bundled CLI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
uniffi-bindgen-cli
here is actually a binary file generated by me based on the content in https://mozilla.github.io/uniffi-rs/tutorial/foreign_language_bindings.html. The code is provided by the documentation, with just the addition of "-cli" to its name. It is used for conveniently calling the uniffi-bindgen generation command with a fixed path in Xcode.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but this binary won't be installed into
$HOME/.cargo/bin/uniffi-bindgen-cli
by default anymore, so we shouldn't recommend that in official documentation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The project does not build as is for me.
Let's rip that out for now and see if maybe we can add it as an example later. That way we can at least land the initial docs now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's caused by the inability to invoke
uniffi-bindgen-cli
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I actually fixed that.
I'd still say let's not overiterate on that, we can add the project in a followup PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned above we don't ship that CLI anymore as is.