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

chore: add info messages to solc install/compile #838

Merged
merged 4 commits into from
Jan 29, 2022

Conversation

odyslam
Copy link
Contributor

@odyslam odyslam commented Jan 28, 2022

Add informational messages to solc installation and compilation.

The compilation of a project could give the appearance of a hanging program because of downloading and installing solc. Informational messages allow the user to know what's happening and why it might take more than expected.

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 for the download step, no need to add extra info elsewhere imo, as one may want to have more control over that message

ethers-solc/src/compile.rs Outdated Show resolved Hide resolved
@gakonst gakonst merged commit 8b9a18c into gakonst:master Jan 29, 2022
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 leaning against this for the simple reason that this is a foundry use case and ethers-solc is a standalone crate that ideally should not print anything unconditionally.

A better solution imo would be to have something like

pub trait SolcProgressMonitor {
   fn on_install(version: &Version) {}
   fn on_installed(version: &Version) {}
   fn on_compile(input: &CompilerInput) {}
}

we could then do something similar to how logging works in general
basically using a global SolcProgressMonitor that calls the callbacks.
This global instance (static mut) can then be instantiated with a StdOutProgressMinitor implementation for example.

this way the user can control, if and how things are logged.

But for the time being this is perfectly fine

@odyslam
Copy link
Contributor Author

odyslam commented Jan 29, 2022

@mattsse -- The reason I made the contributions here is that all these steps are exposed via a single function call in the forge build, as far as I understood it.

@gakonst
Copy link
Owner

gakonst commented Jan 29, 2022

@mattsse agree on the hook for logging to stdout, good idea.

@odyslam odyslam deleted the solc-print branch January 30, 2022 14:47
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.

3 participants