-
-
Notifications
You must be signed in to change notification settings - Fork 131
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!: change CompileAPI to ecosystem based compilers [APE-1322] #1626
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,7 +95,7 @@ jobs: | |
pip install .[test] | ||
|
||
- name: Run Tests | ||
run: pytest -m "not fuzzing" -s --cov=src -n auto --dist loadscope | ||
run: ape test -m "not fuzzing" -s --cov=src --dist loadscope | ||
|
||
fuzzing: | ||
runs-on: ubuntu-latest | ||
|
@@ -117,4 +117,4 @@ jobs: | |
pip install .[test] | ||
|
||
- name: Run Tests | ||
run: pytest -m "fuzzing" --no-cov -s | ||
run: ape test -m "fuzzing" --no-cov -s | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here as well, back to |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -153,7 +153,8 @@ def contracts(self) -> Dict[str, ContractType]: | |
|
||
@property | ||
def _cache_folder(self) -> Path: | ||
folder = self.contracts_folder.parent / ".build" | ||
current_ecosystem = self.network_manager.network.ecosystem.name | ||
folder = self.contracts_folder.parent / ".build" / current_ecosystem | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, so with this change it would have to rebuild for every different ecosystem On the one hand, this may actually be pretty useful for networks like arbitrum that don't support newer VM rules yet (current suggestion is downgrade VM rules to Paris), but would need a way for the ecosystem to basically declare it's okay with another ecosystem type's VM rules Also, I believe ethPM-types will have to change to allow compilation artifacts for multiple ecosystems There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is needed for ethpm-types ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently the data structure is Should be something more like This may take some thinking There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but do we really need this? now if we dont use duplicate plugins per ecosystem, lets say for ethereum, if we dont have solidity and zksolc then contract types shouldnt have any problem in my opinion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The other option is back to what I said before, there's a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldnt really understand this. Manifest and abi files will be synced as we have network option and usage now. Can you give more details on this? What should i do? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pulling this piece out for 0.7.0. |
||
# NOTE: If we use the cache folder, we expect it to exist | ||
folder.mkdir(exist_ok=True, parents=True) | ||
return folder | ||
|
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.
Please revert back to
pytest
,ape test
is used for running ape projects not the framework or plugins.