Skip to content
This repository was archived by the owner on Jan 18, 2023. It is now read-only.

Conversation

bweick
Copy link
Contributor

@bweick bweick commented Jul 3, 2018

No description provided.

@bweick bweick requested a review from a team July 3, 2018 19:57
await subject();

const setTokens = await core.setTokens.callAsync();
expect(setTokens).to.not.include(setToken.address);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can also add an assertion to expected array length

isValidFactory(_factoryAddress)
{
state.validFactories[_factoryAddress] = false;
for (uint256 i = 0; i < state.factories.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we create an address[] helper method for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add to Trello and we can decide. Wanna get this merged within the scope of what it is.

@coveralls
Copy link

coveralls commented Jul 3, 2018

Pull Request Test Coverage Report for Build 500

  • 14 of 14 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 99.701%

Totals Coverage Status
Change from base Build 493: 0.02%
Covered Lines: 243
Relevant Lines: 243

💛 - Coveralls

@bweick bweick force-pushed the brian/factory_and_token_arrays branch 3 times, most recently from 5a69278 to 7877941 Compare July 9, 2018 04:47
@bweick bweick force-pushed the brian/factory_and_token_arrays branch from 30f9ae7 to 48eb564 Compare July 9, 2018 05:29
@bweick bweick merged commit 6c2cf13 into master Jul 9, 2018
@asoong asoong deleted the brian/factory_and_token_arrays branch July 9, 2018 17:19
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