-
Notifications
You must be signed in to change notification settings - Fork 111
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
Release ndk 0.4.0, ndk-glue 0.4.0, ndk-build 0.4.1 #166
Conversation
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.
Can we get a new cargo-apk version too? would love getting the ci fixed too while we're making releases
@dvc94ch it looks like the last change to the |
I guess ndk-build is enough if it's only a patch release. Looking for this commit: |
@dvc94ch Right, yes, a patch release should be enough! |
@dvc94ch Any comment on whether |
it's the safe thing to do. generally I release minor versions whenever I upgrade a dependency. there may be cases were it isn't strictly necessary. but in this case we are leaking types from |
ndk-glue depends on a breaking change in the NDK, and can only be released if that's released under 0.4.0. This, in turn, causes ndk-glue to need a minor bump too despite no other breaking changes, as NDK symbols are used in its public API. Note that ndk-glue 0.3.1 was never properly released as the ndk update requirement outlined above caused an adequate publishing failure.
@dvc94ch Yep, thanks for confirming. Should all be set now, would you mind reviewing once more before we merge this? @iamralpht I'm not sure if we can fully fix #164 as |
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.
thanks!
@@ -1,6 +1,6 @@ | |||
[package] | |||
name = "ndk-build" | |||
version = "0.4.0" | |||
version = "0.4.1" |
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.
checked, a patch is fine
@@ -1,6 +1,6 @@ | |||
[package] | |||
name = "ndk-glue" | |||
version = "0.3.1" | |||
version = "0.4.0" |
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.
this is safe in any case and we already discussed it
@@ -1,6 +1,6 @@ | |||
[package] | |||
name = "ndk" | |||
version = "0.3.0" | |||
version = "0.4.0" |
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.
this is clear from the changelog
@dvc94ch Thanks for checking! Let's get this merged now, I'll be able to monitor the crate during the day in case something happens after all. |
Fixes #164
ndk-glue 0.3.1 depends on a breaking change in the NDK, and can only be released if that's released under 0.4.0.Draft: I'm not sure if this is the right thing to do, or if we should aim for ndk-glue0.4.0
. ndk-glue exposes quite a few NDK symbols in its public API, and runningcargo update
in a crate that uses both (very likely) they'll end up with duplicate crates and incompatible types unless they manually update to ndk 0.4 (which is possibly, but unexpected sudden breakage because semver allows going from 0.3.0 to 0.3.1 without issues).ndk-glue depends on a breaking change in the NDK, and can only be released if that's released under 0.4.0. This, in turn, causes ndk-glue to need a minor bump too despite no other breaking changes, as NDK symbols are used in its public API.
Note that ndk-glue 0.3.1 was never properly released as the ndk update requirement outlined above caused an adequate publishing failure.