refactor(test-helpers): remove test-helpers part one: contracts folder#1977
Open
refactor(test-helpers): remove test-helpers part one: contracts folder#1977
Conversation
Even if this commit looks huge, it is actually quite simple: - It removes `contracts` folder from `@tps/test-helpers` and its content - Moves ADynamicForwarder and DynamicScriptHelpers locally to DotVoting - Change DotVoting imports - Inline IForwarder and rename to solve issues with coverage - Removing pre and post coverage hooks, unnecessary for validation after inlining the interface
ghost
requested review from
This was referenced Feb 10, 2020
topocount
requested changes
Feb 17, 2020
| // TODO: Research why using the @aragon/os version breaks coverage | ||
| import "@aragon/os/contracts/common/IForwarder.sol"; |
Collaborator
There was a problem hiding this comment.
Let's not re-implement this until we decide how to proceed with renaming this import during coverage
| "lint": "solium --dir ./contracts", | ||
| "postcoverage": " sed -i 's+./IForwarder.sol+@aragon/os/contracts/common/IForwarder.sol+g' ../shared/test-helpers/contracts/common/ADynamicForwarder.sol", | ||
| "precommit": "lint-staged", | ||
| "precoverage": "sed -i 's+@aragon/os/contracts/common/IForwarder.sol+./IForwarder.sol+g' ../shared/test-helpers/contracts/common/ADynamicForwarder.sol", |
Collaborator
There was a problem hiding this comment.
It doesn't make sense to remove this since it addresses a bug in the coverage tool. We should upgrade our coverage tool before removing this since it doesn't make sense to create our own duplicated import when it already exists in a dependency we're using
style: remove trailing whitespaces everywhere
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Even if this commit looks huge, it is actually quite simple:
contractsfolder from@tps/test-helpersand its contentinlining the interface
Note: ADynamicForwarder and DynamicScriptHelpers are now part of DotVoting app, that is why coverage was decreased. I decided to not ignore those files, as I think they should be tested as well.