Skip to content

Bump serde to 1.0 #1359

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

Closed
wants to merge 1 commit into from
Closed

Bump serde to 1.0 #1359

wants to merge 1 commit into from

Conversation

Eijebong
Copy link
Contributor

@Eijebong Eijebong commented Jun 10, 2017

Still have all those replaces, but we're almost here !


This change is Reviewable

@Eijebong
Copy link
Contributor Author

Won't work on mac.
I would appreciate it if someone could take on:

I don't have a mac and hope it would just work (and of course it didn't)

@Eijebong Eijebong force-pushed the serde1.0 branch 2 times, most recently from 41df376 to 2445a69 Compare June 11, 2017 21:56
@Eijebong
Copy link
Contributor Author

Thank you so much @killercup for checking/fixing those mac related failures :)

@glennw
Copy link
Member

glennw commented Jun 12, 2017

Holding off on merging after discussing with @nox - please let me know when this is good to go though!

@Eijebong
Copy link
Contributor Author

Well I'm waiting for the euclid 0.14 bump to land in servo and then we're good to go I think (the servo PR is ready modulo euclid)

@glennw
Copy link
Member

glennw commented Jun 14, 2017

We're holding off on merging this while @bholley lands a small change in WR to update rayon. Once that's landed, I'm fine with landing this (if everything else is ready to go).

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #1382) made this pull request unmergeable. Please resolve the merge conflicts.

serde_derive = "0.9"
serde = "0.9"
serde_derive = "1.0"
serde = "1.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the derive feature of serde instead of explicitly depending on serde_derive?

@@ -7,7 +7,7 @@ license = "MPL-2.0"

[dependencies]
base64 = "0.3"
bincode = "1.0.0-alpha2"
bincode = "0.8"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why from a 1.0 alpha to 0.8? Can someone explain to me what's going on with bincode's versioning? It's so confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know anything about bincode's versioning, but 0.8 is the only version supporting serde 1.0, that's why I chose it

@glennw
Copy link
Member

glennw commented Jun 15, 2017

@Eijebong Can all those [replace] sections be removed now, or are we still waiting on some to land?

@Eijebong
Copy link
Contributor Author

@glennw
Copy link
Member

glennw commented Jun 16, 2017

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 0957013 has been approved by glennw

@glennw
Copy link
Member

glennw commented Jun 16, 2017

@bors-servo r-

@glennw
Copy link
Member

glennw commented Jun 16, 2017

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 28f5601 has been approved by glennw

@bors-servo
Copy link
Contributor

⌛ Testing commit 28f5601 with merge 406dbd9...

bors-servo pushed a commit that referenced this pull request Jun 16, 2017
Bump serde to 1.0

Still have all those replaces, but we're almost here !

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1359)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-travis

@glennw
Copy link
Member

glennw commented Jun 16, 2017

@Eijebong The servo-tidy script failed CI due to multiple bitflags crates present.

https://travis-ci.org/servo/webrender/jobs/243487245#L253

@SimonSapin
Copy link
Member

Since the serde update has already landed in some other dependencies but not in Servo, landing serde 1.0 in Servo is blocking getting new unrelated changes from these dependencies into Servo.

Bitflags is small and 0.7.0 (the older version) is used in 5 crates by likely as many maintainers. So let’s not block on deduplicating bitflags. I’ve filed #1391. @Eijebong, please add bitflags to the packages list in [ignore] in servo-tidy.toml, then make sure Tidy is happy by either pushing to this PR and watching Travis-CI or running (preferably in a Python virtualenv) pip install servo-tidy then servo-tidy.

This was referenced Jun 16, 2017
bors-servo pushed a commit that referenced this pull request Jun 16, 2017
Bump serde to 1.0

This is a rebase of #1359 with `bitflags` added to the tidy duplicate crates exception list, and version numbers incremented some more to catch up with `master`. Original work by @Eijebong.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1392)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this pull request Jun 16, 2017
Bump serde to 1.0

This is a rebase of #1359 with `bitflags` added to the tidy duplicate crates exception list, and version numbers incremented some more to catch up with `master`. Original work by @Eijebong.

Fixes #1359.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1392)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants