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

Rename `Config` type to `Build` #226

Merged
merged 2 commits into from Aug 10, 2017

Conversation

Projects
None yet
2 participants
@laumann
Copy link
Contributor

laumann commented Aug 8, 2017

From the discussion it seems that gcc::Build was the preferred name.

Fixes #189

@laumann laumann force-pushed the laumann:rename-config-type branch from 97367cf to a06dcf6 Aug 8, 2017

@alexcrichton

This comment has been minimized.

Copy link
Owner

alexcrichton commented Aug 9, 2017

Thanks! Mind also leaving behind a temporary pub use Build as Config with a #[doc(hidden)] so we can land this in the 0.3 series?

@laumann

This comment has been minimized.

Copy link
Contributor Author

laumann commented Aug 9, 2017

Sure! Should there be a deprecation warning to alert users to change Config to Build?

@laumann

This comment has been minimized.

Copy link
Contributor Author

laumann commented Aug 9, 2017

If I do:

#[doc(hidden)]
#[deprecated(since="0.3.51", notes="gcc::Config has been renamed to gcc::Build")]
pub type Config = Build;

I get deprecation warnings, but that doesn't work with pub use Build as Config

@alexcrichton Which do you prefer?

Add Config as alias for Build
This allows adding deprecation warnings and including the rename in the 0.3
series.

@laumann laumann force-pushed the laumann:rename-config-type branch from a06dcf6 to dc7162f Aug 9, 2017

@alexcrichton

This comment has been minimized.

Copy link
Owner

alexcrichton commented Aug 10, 2017

@laumann ah unfortunately I think they'll effectively be the same which is to say functionally equivalent and #[deprecated] likely ignored on both.

In any case though this looks great to me, thanks!

@alexcrichton alexcrichton merged commit 9a09971 into alexcrichton:master Aug 10, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@laumann

This comment has been minimized.

Copy link
Contributor Author

laumann commented Aug 10, 2017

ah unfortunately I think they'll effectively be the same which is to say functionally equivalent and #[deprecated] likely ignored on both.

I tried using gcc::Config in gcc-test/build.rs and only with the type alias did I get a deprecation warning when running cargo test (not with pub use).

Like this:

$ pwd
/home/tj/rust/gcc-rs/gcc-test
$ cargo check
   Compiling gcc v0.3.51 (file:///home/tj/rust/gcc-rs)
   Compiling gcc-test v0.1.0 (file:///home/tj/rust/gcc-rs/gcc-test)
warning: use of deprecated item: gcc::Config has been renamed to gcc::Build
  --> build.rs:12:5
   |
12 |     gcc::Config::new()
   |     ^^^^^^^^^^^^^^^^
   |
   = note: #[warn(deprecated)] on by default
    Finished dev [unoptimized + debuginfo] target(s) in 2.19 secs

But I don't know if there is anything special about gcc-test...

@laumann laumann deleted the laumann:rename-config-type branch Aug 10, 2017

@alexcrichton

This comment has been minimized.

Copy link
Owner

alexcrichton commented Aug 10, 2017

Oh hey! I guess some bugs in rustc have gotten fixed since I last looked!

@laumann

This comment has been minimized.

Copy link
Contributor Author

laumann commented Aug 10, 2017

Always a nice surprise :-) But should deprecation on pub use Foo as Bar also generate a warning?

@alexcrichton

This comment has been minimized.

Copy link
Owner

alexcrichton commented Aug 10, 2017

Last I checked it didn't, but maybe it's changed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.