Skip to content
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.

Sign up for GitHub

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

refactor(test-helpers): remove test-helpers part one: contracts folder #1977

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

refactor(test-helpers): remove test-helpers part one: contracts folder #1977

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b8a5cd
refactor(test-helpers): remove test-helpers part one: contracts folder
ottodevs Feb 10, 2020
1c3d05e
style: remove trailing whitespaces everywhere
ottodevs Feb 10, 2020
e427368
Merge pull request #1978 from AutarkLabs/remove-trailing-whitespaces
ottodevs Mar 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/dot-voting/contracts/DotVoting.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import "@aragon/apps-shared-minime/contracts/MiniMeToken.sol";

// TODO: Revert import path when changes get merged into aragon/os
// import "@aragon/os/contracts/common/ADynamicForwarder.sol";
import "@tps/test-helpers/contracts/common/ADynamicForwarder.sol";
import "./lib/ADynamicForwarder.sol";


contract DotVoting is ADynamicForwarder, AragonApp {
Expand Down
18 changes: 15 additions & 3 deletions ...rs/contracts/common/ADynamicForwarder.sol → ...oting/contracts/lib/ADynamicForwarder.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,21 @@ import "@aragon/os/contracts/lib/math/SafeMath.sol";
import "@aragon/os/contracts/lib/math/SafeMath64.sol";

// TODO: Use @aragon/os/contracts/ version when it gets merged
import "../evmscript/DynamicScriptHelpers.sol";
import "./DynamicScriptHelpers.sol";
// TODO: Research why using the @aragon/os version breaks coverage
import "@aragon/os/contracts/common/IForwarder.sol";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not re-implement this until we decide how to proceed with renaming this import during coverage

// import "@aragon/os/contracts/common/IForwarder.sol";

interface InterfacedForwarder {
function isForwarder() external pure returns (bool);

// TODO: this should be external
// See https://github.com/ethereum/solidity/issues/4832
function canForward(address sender, bytes evmCallScript) public view returns (bool);

// TODO: this should be external
// See https://github.com/ethereum/solidity/issues/4832
function forward(bytes evmCallScript) public;
}

/**
* @title ADynamicForwarder App
Expand All @@ -21,7 +33,7 @@ import "@aragon/os/contracts/common/IForwarder.sol";
*/


contract ADynamicForwarder is IForwarder {
contract ADynamicForwarder is InterfacedForwarder {
using DynamicScriptHelpers for bytes;
using SafeMath for uint256;
using SafeMath64 for uint64;
Expand Down
0 ...tracts/evmscript/DynamicScriptHelpers.sol → ...ng/contracts/lib/DynamicScriptHelpers.sol
View file
Open in desktop
File renamed without changes.
2 changes: 0 additions & 2 deletions apps/dot-voting/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@
"frontend": "npm run sync-assets && parcel app/index.html --port 4444",
"ganache-cli:test": "sh ../../node_modules/@aragon/test-helpers/ganache-cli.sh",
"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",
Copy link
Collaborator

@topocount topocount Feb 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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

"prepublishOnly": "truffle compile",
"publish:cd": "../../shared/deployments/check-publish.sh",
"publish:http": "npm run build:script && yes | aragon apm publish major --files dist --http localhost:4444 --http-served-from ./dist --propagate-content false --skip-confirmation true",
Expand Down
Loading