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

feat(solc): add workspace utils #678

Merged
merged 8 commits into from
Dec 12, 2021
Merged

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Dec 11, 2021

Motivation

Add utils for ethers-solc to easily create temp workspaces we can also use in foundry.

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@mattsse mattsse changed the title feat(solc): add workspace mocking feat(solc): add workspace utils Dec 12, 2021
@mattsse mattsse marked this pull request as ready for review December 12, 2021 12:15
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.

LGTM - small nits / questions

}

/// Copies a single file into the given dir
pub fn copy_file(&self, source: impl AsRef<Path>, target_dir: impl AsRef<Path>) -> Result<()> {
Copy link
Owner

Choose a reason for hiding this comment

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

Think this shouldn't be needed?

Suggested change
pub fn copy_file(&self, source: impl AsRef<Path>, target_dir: impl AsRef<Path>) -> Result<()> {
pub fn copy_file(source: impl AsRef<Path>, target_dir: impl AsRef<Path>) -> Result<()> {

Comment on lines 59 to 65
/// Returns the handle to the tempdir and the project
///
/// NOTE: the `TempDir` object deletes its directory on drop, also removing all the project's
/// content
pub fn split(self) -> (Project<T>, TempDir) {
(self.inner, self.root)
}
Copy link
Owner

Choose a reason for hiding this comment

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

This seems unused? Is it needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, used that before adding more utility functions, removed now

}

/// Copies all content of the source dir into the target dir
pub fn copy_dir(&self, source: impl AsRef<Path>, target_dir: impl AsRef<Path>) -> Result<()> {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
pub fn copy_dir(&self, source: impl AsRef<Path>, target_dir: impl AsRef<Path>) -> Result<()> {
pub fn copy_dir(source: impl AsRef<Path>, target_dir: impl AsRef<Path>) -> Result<()> {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved to standalone function, same as copy_file

Comment on lines 130 to 139
let cache = tmp_dir.path().join("cache");
let cache = cache.join(SOLIDITY_FILES_CACHE_FILENAME);

let paths = ProjectPathsConfig::builder()
.cache(cache)
.sources(root.join("contracts"))
.artifacts(root.join("artifacts"))
.lib(root.join("node_modules"))
.root(root)
.build()?;
Copy link
Owner

Choose a reason for hiding this comment

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

Can't you use ProjectPathsConfig::hardhat instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice, this simplifies it quite a bit

ethers-solc/src/project_util.rs Outdated Show resolved Hide resolved
.build()
.unwrap();
let paths = ProjectPathsConfig::builder().sources(root.join("src")).lib(root.join("lib"));
let project = TempProject::<MinimalCombinedArtifacts>::new(paths).unwrap();
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe this could be done with TempProject::<_>::dapptools()?

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 we're essentially setting the project root to ./test-data/.. instead of creating a totally new, empty temp project that TempProject::<_>::dapptools() would do.
in these cases we'd need to use TempProject::new and pass in the preconfigured ProjectPaths,

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.

great!

@gakonst gakonst merged commit 4c67793 into gakonst:master Dec 12, 2021
meetmangukiya pushed a commit to meetmangukiya/ethers-rs that referenced this pull request Mar 21, 2022
* feat(config): support omptimizer details

* bump ethers

* chore: bump ethers

Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>
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.

2 participants