Add serde support and bump version to 0.3.0#31
Conversation
mbrubeck
left a comment
mbrubeck
left a comment
There was a problem hiding this comment.
This looks good. Just some minor comments:
Cargo.toml
Outdated
| matches = "0.1" | ||
| serde = {version = ">=0.8", optional = true} | ||
| serde_test = {version = ">=0.8", optional = true} | ||
| serde_derive = {version = ">=0.8", optional = true} |
There was a problem hiding this comment.
Please make these dependencies `">=0.8, <2.0".
src/deprecated.rs
Outdated
| /// Find the level runs within a line and return them in visual order. | ||
| /// | ||
| /// NOTE: This implementation is incomplete. The algorithm needs information about the text, | ||
| /// including original BidiClass property of each character, to be able to perfor correctly. |
src/level.rs
Outdated
| pub struct Level(u8); | ||
|
|
||
| #[cfg(feature = "with_serde")] | ||
| #[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Serialize, Deserialize)] |
There was a problem hiding this comment.
Could #[cfg_attr(feature = "with_serde", derive(Serialize, Deserialize))] work here?
Add `with_serde` feature, which implements serde for the new `struct Level`, mainly used in `servo`, supporting serde `0.8`, `0.9` and `1.0`. Add tests for the `with_serde` feature. The `serde_tests` modules only works for `serde:>=1.0`, though. `servo` has dependency on the loose implementation of `visual_runs()`, which couldn not be improved without breaking the API, as it needs more information to process the levels correctly, and the call-site in `servo` does not have all the information needed and needs a non-trivial change to work with the new improved version. Therefore, I have moved the old version to a `deprecated` module, to be used for now until `servo` is fixed and we drop the old implementation. Bump version to `0.3.0`, as we now ready for a release: can build `servo` (patch ready) and `idna` crates.
|
Okay, addressed all the comments, plus merging a couple of consequent |
|
Review status: 0 of 4 files reviewed at latest revision, 3 unresolved discussions. Cargo.toml, line 20 at r1 (raw file): Previously, mbrubeck (Matt Brubeck) wrote…
Done. src/deprecated.rs, line 17 at r1 (raw file): Previously, mbrubeck (Matt Brubeck) wrote…
Done. Comments from Reviewable |
|
@bors-servo r+ |
|
📌 Commit efa3b2a has been approved by |
Add serde support and bump version to 0.3.0 Add `with_serde` feature, which implements serde for the new `struct Level`, mainly used in `servo`, supporting serde `0.8`, `0.9` and `1.0`. Add tests for the `with_serde` feature. The `serde_tests` modules only works for `serde:>=1.0`, though. `servo` has dependency on the loose implementation of `visual_runs()`, which couldn not be improved without breaking the API, as it needs more information to process the levels correctly, and the call-site in `servo` does not have all the information needed and needs a non-trivial change to work with the new improved version. Therefore, I have moved the old version to a `deprecated` module, to be used for now until `servo` is fixed and we drop the old implementation. Bump version to `0.3.0`, as we now ready for a release: can build `servo` (patch ready) and `idna` crates. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/unicode-bidi/31) <!-- Reviewable:end -->
|
☀️ Test successful - status-travis |
|
@mbrubeck, how these packages get posted on crates.io? Looks like it's manual. Could you please publish Also, are there any projects automating this: releasing on crates.io from travis based on git tags? |
|
Tagged and published. @behnam, would you like to be added as a contributor/owner of this package so that you can publish new versions yourself? |
|
Thanks, @mbrubeck. Well, why not! Would makes things easier for both of us! :) |
3 participants
Add
with_serdefeature, which implements serde for the newstruct Level, mainly used inservo, supporting serde0.8,0.9and1.0.Add tests for the
with_serdefeature. Theserde_testsmodules onlyworks for
serde:>=1.0, though.servohas dependency on the loose implementation ofvisual_runs(),which couldn not be improved without breaking the API, as it needs more
information to process the levels correctly, and the call-site in
servodoes not have all the information needed and needs a non-trivialchange to work with the new improved version. Therefore, I have moved the
old version to a
deprecatedmodule, to be used for now untilservois fixed and we drop the old implementation.
Bump version to
0.3.0, as we now ready for a release: can buildservo(patch ready) andidnacrates.This change is