search

georust / geojson

Library for serializing the GeoJSON vector GIS file format
https://crates.io/crates/geojson
Apache License 2.0
280 stars 60 forks source link

Release 1.0 #244

Open maxammann opened 3 months ago

maxammann commented 3 months ago

It seems this crate can be thought of as "done" as it implements a spec. Are there any blockers that would need to be done before this can reach 1.0?

I think log and geo_types are both blockers because they are not 1.0 yet.

urschrei commented 3 months ago

The main question is whether we're happy with perf, which is related to the way we handle errors: https://github.com/georust/geojson/issues/197, https://github.com/georust/geojson/issues/176, https://github.com/georust/geojson/issues/180

michaelkirk commented 3 months ago

I think log and geo_types are both blockers because they are not 1.0 yet.

Can you articulate why depending on a non-1.0 dependency blocks us from releasing a 1.0? I'm not aware of why it would.

michaelkirk commented 3 months ago

A bigger thing that I might be worried about before 1.0 is making the data types opaque. Currently they are all just aliases for Vec.

So that we can do things like: https://github.com/georust/geojson/pull/222/files#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759L427

michaelkirk commented 3 months ago

I think log and geo_types are both blockers because they are not 1.0 yet.

Can you articulate why depending on a non-1.0 dependency blocks us from releasing a 1.0? I'm not aware of why it would.

Firstly, semantic version lawyering is not my strong suit, but in a moment of synchronicity, I may have answered my own question, when it was asked differently in another context:

https://github.com/pka/http-range-client/pull/8#issuecomment-2284841165

Since log isn't used in our public API anywhere, I think it's not relevant, so long as we trust the maintainers of log to not make backwards incompatible changes without bumping the 0.Y.0.

Technically anything goes when you're 0.y.z,

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

But I'm comfortable betting on the log maintainers to bump 0.Y.0 when they make backwards incompatible changes, as they have for the last many years.

geo-types is a little tricker.

Since we're implementing traits on geo-types, it's part of our public API, so semver breakage in geo-types means breakage in geojson (right?). Although geo-types isn't yet 1.0, like log, it's relatively stable and we are careful about properly bumping geo-types 0.Y.0 whenever we make a change that is not backwards compatible.

So if we want to release geojson 1.0 before geo-types 1.0 (which we could), geojson would get an extra major version bump if/when geo-types finally ever gets to 1.0.

maxammann commented 3 months ago

Firstly, semantic version lawyering is not my strong suit, but in a moment of synchronicity, I may have answered my own question, when it was asked differently in another context:

pka/http-range-client#8 (comment)

Since log isn't used in our public API anywhere, I think it's not relevant, so long as we trust the maintainers of log to not make backwards incompatible changes without bumping the 0.Y.0.

Technically anything goes when you're 0.y.z,

I thought I would find some better list than this but I think the rule is that all your dependencies need to be 1.0, even if they are not public facing. I could be very wrong on this though. Imaging that log adds a rust-version tag to their Cargo.toml That would break compiling geosjon with the older Rust version. That would be a breaking (Cargo) change imo.

Releasing a 1.0 for log seems to have stalled: https://github.com/rust-lang/log/issues/375

So if we want to release geojson 1.0 before geo-types 1.0 (which we could), geojson would get > an extra major version bump if/when geo-types finally ever gets to 1.0.

That could make sense yeah. I'm a bit mixed on the situation of depending on non stable crates in a stable crate. I'll try to find more information of the communities opinion on that.

maxammann commented 3 months ago

Found the reference! https://rust-lang.github.io/api-guidelines/necessities.html#c-stable

btw, the reason is that I want to create a crate based on this and though: Hey! this could be come stable pretty quickly!

After all, this migth be more a geo-types discussion, first and foremost :)

michaelkirk commented 3 months ago

Found the reference! https://rust-lang.github.io/api-guidelines/necessities.html#c-stable

My read of this is we don't need to worry about log, since it's not a public dependency.

Imaging that log adds a rust-version tag to their Cargo.toml That would break compiling geosjon with the older Rust version. That would be a breaking (Cargo) change imo.

WRT to a dependency bumping MSRV specifically: It's true — that could break someone's build. Much ink has been spilled on whether MSRV bumps should result in bumping your library's major version. I see both sides, but agree with the majority - namely that MSRV should not constitute a breaking release: https://github.com/rust-lang/api-guidelines/discussions/231

So I think geo-types is the only relevant dependency in terms of 1.0.