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

fix: docsrs builds final #2235

Merged
merged 4 commits into from Mar 8, 2023
Merged

fix: docsrs builds final #2235

merged 4 commits into from Mar 8, 2023

Conversation

DaniPopes
Copy link
Collaborator

@DaniPopes DaniPopes commented Mar 6, 2023

Motivation

For some reason (I believe because the build process isn't normal and maybe our resolution doesn't work with the given environment) the docs.rs builds always return ethers as the crate name (default). Tested multiple times on a local docs.rs instance and I can 99.9999999% confirm that this PR solves it by doing the following:

  • re-export the internal crate as ethers
  • export core, contract and providers at the top level
  • rustc flags also have to be set to --cfg docsrs (for #[cfg(docsrs)] on the re-exports)
    • this is technically not necessary now because I removed the cfgs, just to be safe

this needs to be done in every (release) crate that uses a macro that uses ethers_core::macros::*, which right now includes contract and middleware.

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog
  • Breaking changes

@DaniPopes DaniPopes marked this pull request as draft March 6, 2023 05:06
@DaniPopes DaniPopes marked this pull request as ready for review March 6, 2023 05:10
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit skeptical that messing with module alias just to get docs working is the right way to fix this.

Cargo.toml Show resolved Hide resolved
Comment on lines +46 to +57
#[doc(hidden)]
#[allow(unused_extern_crates)]
extern crate self as ethers;

#[doc(hidden)]
pub use ethers_contract as contract;

#[doc(hidden)]
pub use ethers_core as core;

#[doc(hidden)]
pub use ethers_providers as providers;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I'm not sure I like that these are now always there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you have a better way to find ethers crate names then go ahead, this is a patch for builds when that is not possible, and only for our internal crates

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a better solution for this, and seems like this is a proper fix.
But let's document why we're doing this

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with Matt that it sucks we need to do this, but let's roll with it

@gakonst gakonst merged commit f9086e7 into gakonst:master Mar 8, 2023
10 of 12 checks passed
@DaniPopes DaniPopes deleted the fix/docs-rs2 branch March 9, 2023 05:58
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

Successfully merging this pull request may close these issues.

None yet

3 participants