-
Notifications
You must be signed in to change notification settings - Fork 209
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
Update axum to v0.8.0 #1269
Update axum to v0.8.0 #1269
Conversation
@@ -4,12 +4,12 @@ version = "0.1.0" | |||
edition = "2021" | |||
|
|||
[dependencies] | |||
axum = { version = "0.7", features = ["multipart"] } | |||
axum = { version = "0.8.0", features = ["multipart"] } |
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 think leaving this at 0.8 instead of 0.8.0 is better. As far as I'm aware axum doesn't break on patch increases and there's already 0.8.1 out. This is not only for this case, but all axum versions in this 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.
0.8
and 0.8.0
are equivalent (see https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html)
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.
Yeah, as the default resolution style is with the ^ which indicates it allows versions between 0.8.0 - 0.8.x
. If it was declared with = it would explicitly state the specific version. Though above, this is just matter of style, I deem to remove the unnecessary zeros but the effect is same.
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.
THX, TIL.
Either way, it probably should already be the newest one.
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.
0.8.1 was only a small readme change, so probably not worth the extra constraint :)
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.
Yup, if you do a clean build with it, it will and should pull the latest available version from crates 🙂
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.
Looks quite nice. Thanks for putting the effort. 🙂
Still missing the famous CHANGELOG.md
entries 😉
I don't have the time to put much effort into this project at the moment but perhaps when I get my work sorted out I have more time to focus on this one as well. 😞
utoipa-axum/src/router.rs
Outdated
{ | ||
String::from(path).replace('}', "").replace('{', ":") | ||
} | ||
|
||
#[inline] | ||
fn path_template<S: AsRef<str>>(path: S) -> String { |
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 can most likely remove this path_template
fn as well since it is used to counter the colonized syntax and translate colonized paths to path template syntax, but since axum 0.8 comes with path template syntax this because redundant 🤔
@@ -4,12 +4,12 @@ version = "0.1.0" | |||
edition = "2021" | |||
|
|||
[dependencies] | |||
axum = { version = "0.7", features = ["multipart"] } | |||
axum = { version = "0.8.0", features = ["multipart"] } |
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.
Yeah, as the default resolution style is with the ^ which indicates it allows versions between 0.8.0 - 0.8.x
. If it was declared with = it would explicitly state the specific version. Though above, this is just matter of style, I deem to remove the unnecessary zeros but the effect is same.
axum 0.8 uses the same syntax as OpenAPI now :)
Otherwise there is no way to fixing this example ahead of the release...
axum 0.8 uses the same syntax as OpenAPI now :)
feedback should be addressed and CI is green 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.
Seems good, probably should copy paste the CHANGELOG.md entry from
utoipa-swagger-uito
utoipa-redocand
utoipa-scalarand
utoipa-rapidoc` as well.
Great, thanks for the hard work 👍 |
utoipa = { path = "../../utoipa", features = ["axum_extras"] } | ||
utoipa-swagger-ui = { path = "../../utoipa-swagger-ui", features = ["axum"] } | ||
utoipa-axum = { path = "../../utoipa-axum" } |
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.
Maybe I'm wrong here an the docs I found are outdated, but I think this will create a problem during publishing:
This should probably include the path AND the version from crates.io.
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.
Note: crates.io does not allow packages to be published with dependencies on code outside of crates.io, except for dev-dependencies. See the Multiple locations section for a fallback alternative for git and path dependencies.
Yes that is a case when you use the local path dependencies within crates that are supposed to be released. However this is an example crate which is not released thus not creating issues there.
This path change does not affect releasability of the referenced crates, allowing them to be released as normal but only prohibiting release of the crate using the references without version or git ref.
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.
Oh, I missed which Cargo.toml this was in - sorry. Thanks for clarifying.
@juhaku any chance we could get this released in the next couple of days? happy to help, if I can :) |
Sure thing, there is not much to do actually, as I got fixed one bug that was caused by changes introduced to generic schemas behavior. Only thing to do is the define the next versions and update the CHANGELOGS with them and update Cargo.toml files with new versions. After pushing the CI does the rest (after pressing the publish button in releases) 🙂 |
This PR updates our
axum
dependencies to v0.8.0 and adjusts to the breaking changes in this release.Specifically, it removes the
colonized_params()
fn from the router inutoipa-axum
, since axum 0.8 now uses the same path syntax as the OpenAPI spec. It also adds a couple ofSync
constraints.Until
axum-test
has been updated to support the new version we won't be able to update this yet, which is why I'm opening this PR in draft mode. This PR currently also includes #1268 to prevent merge conflicts.