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

Conversation

@felix2feng
Copy link
Contributor

@felix2feng felix2feng commented Nov 22, 2018

  • Non functionality changing code touch ups

@coveralls
Copy link

coveralls commented Nov 22, 2018

Pull Request Test Coverage Report for Build 2996

  • 21 of 21 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 2972: 0.0%
Covered Lines: 666
Relevant Lines: 666

💛 - Coveralls

Copy link
Contributor

@asoong asoong left a comment

Choose a reason for hiding this comment

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

LGTM

// Create combined token Array
address[] memory oldComponents = currentSetInstance.getComponents();
address[] memory newComponents = nextSetInstance.getComponents();

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I prefer to not have the space when newComponents was invoked so that it can be called in the next line. Also makes the comment orphaned

* unit of the two sets.
*/
function auctionSetUp()
function setUpAuctionDataStructures()
Copy link
Contributor

Choose a reason for hiding this comment

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

setupAuctionParameters maybe? dataStructures is goofy

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 since it does more than just set up the arrays it also defines the minimumBid too.

@felix2feng felix2feng merged commit 1109bfc into master Nov 26, 2018
@felix2feng felix2feng deleted the felix/rebalancing-set-token-cleanup branch November 26, 2018 04:37
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.

5 participants