Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

chore(deps): bump and use workspace dependencies #2222

Merged
merged 47 commits into from
Mar 16, 2023

Conversation

DaniPopes
Copy link
Collaborator

Motivation

Closes #1963

Solution

See commit log

PR Checklist

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

@gakonst
Copy link
Owner

gakonst commented Mar 13, 2023

sorry need to bump versions to 2.0 everywhere

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.

fine w me - want +1 from @mattsse

@@ -147,9 +165,6 @@ jobs:
- uses: Swatinem/rust-cache@v2
- name: Check
run: cargo check --workspace --target wasm32-unknown-unknown
# TODO: [#2191](https://github.com/gakonst/ethers-rs/issues/2191)
Copy link
Owner

Choose a reason for hiding this comment

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

is this no longer a problem?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm i forget why i removed that, it still is with the features

Comment on lines 337 to +338
#[cfg(target_arch = "wasm32")]
wasm_timer::Delay::new(next_backoff)
.await
.map_err(|_| RetryClientError::TimerError)?;
futures_timer::Delay::new(next_backoff).await;
Copy link
Owner

Choose a reason for hiding this comment

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

is this correct? if so, should we just use it in both cases?

@@ -70,7 +70,7 @@ pub(crate) fn take_solc_installer_lock() -> std::sync::MutexGuard<'static, ()> {
/// A list of upstream Solc releases, used to check which version
/// we should download.
/// The boolean value marks whether there was an error accessing the release list
#[cfg(all(feature = "svm-solc"))]
#[cfg(all(feature = "svm-solc", not(target_arch = "wasm32")))]
Copy link
Owner

Choose a reason for hiding this comment

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

Does anything in ethers-solc work inside of WASM, given it's calling the binary? Should we just make the crate not compile with ethers-rs if wasm is not configured?

@@ -1,10 +1,14 @@
[package]
Copy link
Owner

Choose a reason for hiding this comment

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

Can we make cargo release not go over the examples crates if possible? Right now it thinks all the examples should be published

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

doesn't publish = false do the trick? What's the command you run to release

Comment on lines +1 to +5
[package]
name = "ethers"
authors = ["Georgios Konstantopoulos <[email protected]>"]
readme = "../README.md"
description = "A complete Ethereum and Celo Rust library"
Copy link
Owner

Choose a reason for hiding this comment

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

Are we OK with pulling ethers back down a directory? I initially had pulled it up to surface the examples, but seems fine now given we have separate examples crates

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think so, also this way it doesn't pull in every folder like .github when packaging the ethers crate for release

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.

haven't used workspace deps yet, but this actually seems easier to manage.

so supportive of 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.

ok let's do this

@gakonst gakonst merged commit 439a0c7 into gakonst:master Mar 16, 2023
@DaniPopes DaniPopes deleted the chore/deps branch March 16, 2023 20:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support workspace dependencies
3 participants