-
-
Notifications
You must be signed in to change notification settings - Fork 154
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
feat: add --exclude-compiler flag to ape compile command #2494
base: main
Are you sure you want to change the base?
Conversation
src/ape_compile/_cli.py
Outdated
@@ -42,6 +42,11 @@ def _include_dependencies_callback(ctx, param, value): | |||
help="Also compile dependencies", | |||
callback=_include_dependencies_callback, | |||
) | |||
@click.option( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may also be useful in the ape pm compile
command. We could potentially add this to the ape.cli.options
module so it can be re-used.
src/ape_compile/_cli.py
Outdated
@@ -42,6 +42,11 @@ def _include_dependencies_callback(ctx, param, value): | |||
help="Also compile dependencies", | |||
callback=_include_dependencies_callback, | |||
) | |||
@click.option( | |||
"--exclude-compiler", | |||
type=click.Choice(["solidity", "vyper"], case_sensitive=False), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be a click.Choice. There are more compilers besides solidity
and vyper
. With Ape's plugin system, there isn't a fixed number of compilers.
A normal str
type is just find.
f14b068
to
d51b037
Compare
@@ -449,11 +449,15 @@ def _get_needs_compile(self, paths: Iterable[Union[Path, str]]) -> Iterable[Path | |||
else: | |||
yield path | |||
|
|||
def _compile_contracts(self, paths: Iterable[Union[Path, str]]): | |||
def _compile_contracts( | |||
self, paths: Iterable[Union[Path, str]], exclude_compiler: Optional[str] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this should be a list of compiler names
return click.option( | ||
"--exclude-compiler", | ||
help="Exclude a specific compiler from the compilation process", | ||
type=str, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should do is_multiple=True
here so multiple compilers can be excluded
What I did
Add a new flag to the
ape compile
command that allows to specify a compiler to be excluded from the compilation:fixes: #2259
How I did it
Before invoking compiler's
compile
method , the code checks ifexclude-compiler
parameter matches the current compiler. If it does, the method skips that compiler and moves to the next one.How to verify it
If you think is correct I could proceed with the tests.