-
Notifications
You must be signed in to change notification settings - Fork 346
Allow serde 0.8 and remove use of compiler plugins #216
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
Conversation
The serde feature is actually used outside of Servo, and it would break at least feeds-to-pocket. |
This would also break wolfram-alpha-rs and xkcd-rs. @FraGag and @indiv0: Would you mind having to relax the serde bound in your own crates at the same time as we merge this here, so that we avoid a 2.0.0 bump in rust-url? |
Well this is annoying. If I relax the requirement on serde in feeds-to-pocket but stay on url 1.1.1, then feeds-to-pocket picks up serde 0.8.x while url stays on serde 0.7.x, which causes build errors. On the other hand, if I don't relax the requirement on serde in feeds-to-picket but update url to your branch, then it's the other way around: feeds-to-pocket stays on 0.7.x while url picks up 0.8.x, which also causes build errors. If I update both at once, then it's fine. :\ If rust-lang/cargo#2064 was fixed, then this wouldn't be an issue. feeds-to-pocket is an executable, not a library, so the package comes with its Cargo.lock, and unless a user runs |
@FraGag Given feeds-to-pocket is an executable, there is no need for relaxed requirements there, even if you update rust-url after this is published, it won't make it use serde 0.8 as long as you don't also update serde AFAIK. |
Even though publishing this change can break projects that don’t have a As such, I believe this does not qualify as a breaking API change, and so 1.2.0 rather than 2.0.0 is fine. (Or even 1.1.2, but let’s not split hairs about this, Cargo treats them the same.) @bors-servo r+ |
📌 Commit 9f2f582 has been approved by |
Allow serde 0.8 and remove use of compiler plugins <!-- Reviewable:start --> This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/216) <!-- Reviewable:end -->
☀️ Test successful - travis |
Just as a heads up (in particular to @nox): I just reinstalled feeds-to-pocket v0.1.1 locally with So this did end up being a breaking change, but I agree with @SimonSapin that this kind of change should not be treated as a breaking change. Rather, Cargo could and should be improved such that situations like this do not cause breakage in the future (see e.g. rust-lang/cargo#2064, which I also mentioned above). |
This change is