Skip to content
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

Publish 1.0 #16

Closed
matklad opened this issue Nov 13, 2019 · 7 comments
Closed

Publish 1.0 #16

matklad opened this issue Nov 13, 2019 · 7 comments

Comments

@matklad
Copy link
Collaborator

matklad commented Nov 13, 2019

This seems to be a really great foundational crate with a rather narrow and well defined API. Maybe just publish 1.0, while there are relatively few users, to avoid upgrade pains?

@cuviper
Copy link
Owner

cuviper commented Dec 14, 2019

I would be OK with that -- especially with #19 that will allow you to check pretty much anything. Would you care to review that, and maybe audit the API in general for 1.0? (Perhaps you already have...)

@matklad
Copy link
Collaborator Author

matklad commented Dec 14, 2019

I've looked through the API surface of the crate, everything seems reasonable! The only two non super obvious bits were:

  • rerun_path and rerun_env names are not self-explanatory. return_if_x_changed would be more obvious at the call site, but it probably doesn't make sense to do a rename, as it's a really minor issue, the explicit version is significantly more verbose (though this shouldn't matter much for build scripts?), and we already have an existing API.

  • Error type does not implement the new Source method, but it's not unclear how to support that, without some hockey bootstraping scheme for autocfg itself.

@matklad
Copy link
Collaborator Author

matklad commented Dec 14, 2019

Oh, in Cargo.toml, license = "Apache-2.0/MIT" I think should be spelled as license = "Apache-2.0 OR MIT", but I am not sure if Cargo 1.0 supports this.

@cuviper
Copy link
Owner

cuviper commented Jan 8, 2020

  • rerun_path and rerun_env names are not self-explanatory. return_if_x_changed would be more obvious at the call site, but it probably doesn't make sense to do a rename, as it's a really minor issue, the explicit version is significantly more verbose (though this shouldn't matter much for build scripts?), and we already have an existing API.

I see your point, but yes, the added verbosity would seem unfortunate. I think it's ok to excuse the terse names because the API surface is so small.

  • Error type does not implement the new Source method, but it's not unclear how to support that, without some hockey bootstraping scheme for autocfg itself.

Yeah, I think that bootstrap complexity would not be justified. I expect most cases will just be unwrapped anyway, so the Debug implementation is really what matters.

Oh, in Cargo.toml, license = "Apache-2.0/MIT" I think should be spelled as license = "Apache-2.0 OR MIT", but I am not sure if Cargo 1.0 supports this.

AFAICT Cargo doesn't validate the license at all, leaving it to the registry:
https://github.com/rust-lang/cargo/blob/7059559d71de3fffe8c8cb81e32f323454aa96c5/src/cargo/core/manifest.rs#L75-L79

@matklad
Copy link
Collaborator Author

matklad commented Jan 8, 2020

Yeah, indeed, Cargo doesn’t do validation here. So, we should change this to OR, because that’s what standard says.

rust-lang/cargo#2039

@cuviper
Copy link
Owner

cuviper commented Jan 9, 2020

Published!

@cuviper cuviper closed this as completed Jan 9, 2020
@matklad
Copy link
Collaborator Author

matklad commented Jan 9, 2020

@cuviper I think it might be the good idea to announce the crate on urlo/r/rust? I have a feeling that this crate is barely known (19 ⭐, and I know it only because I was browsing cargo's issue tracker the other day and noticed alexcrichton mentioning it in passing). At the same time, I think there will be ecosystem-wide goodness if everyone converges to this crate.

Also, "MSRV is 1.0.0" is just to cool to not brag about :-)

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

No branches or pull requests

2 participants