fix(enums): Add pub use/namespacing where required #118
Conversation
iterion
changed the title
|
@iterion Might want to hold off on the 2nd commit pending a decision on #113. We were trying to minimize dependency churn but it seems to have caused unintended side effects so we may be reverting the changes. I'm assuming the travis failure is just it running an old rustc which still has libtime? I haven't looked at other projects since the enum namespacing changes, is re-exporting in the parent namespace common? Perhaps it's cleaner to just break/depreciate current code and push users to import variants themselves if they don't wish to qualify them? |
|
Ahh, didn't see #113, I'll add my thoughts on that there. I'll pull out the second commit for now, it may break the build as I think some of the nickel-org forks are also broken because of the enum namespace refactor.
I'm not sure of the correct way to approach this. In the few projects I've looked at since the change it seems they're just re-exporting. I handled the MediaType enum differently since it was the easiest (maybe the only) approach with the macro that is being used there. |
iterion
force-pushed
the
topic/enum-namespacing
branch
from
64c8367
to
7f5a513
Compare
Yeah, it seems rustc builds are still being released with libtime but it's been deprecated. Locally my solution has just been to delete the libtime files that are distributed with rustc. I'm sure there is a better way to handle it. |
|
@iterion yeah, can you add to Cargo.toml? [dependencies.time]
git = "https://github.com/rust-lang/time" |
BREAKING CHANGE: This change will require downstream users to reference mime types using their namespace. i.e. MediaType::Json I added pub use in places where the enums wouldn't be poluting user namespaces with a large amount of names.
iterion
force-pushed
the
topic/enum-namespacing
branch
from
7f5a513
to
9e2b4ea
Compare
|
Added, thanks. |
|
@iterion I updated the cargo file and amended your commit to add a missed update in (Started before your latest commit change but had dependencies to fix, should be a working master now) So, this is merged but github probably wont show it since you changed the commit, but thanks! :) |
|
No worries, thanks for the merge! |
BREAKING CHANGE: This change will require users to reference
mime types using their namespace. i.e. MediaType::Json.
Rust enums were updated in the latest nightly (rust-lang/rust#18973).
I added pub use in places where the enums wouldn't be poluting user
namespaces with a large amount of names.
I also switched to the upstreams of rust-http and rust-url. It seems that the nickel-org forks no longer diverge from their upstreams. I'm happy to split this change into a second PR.