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

Contracts/feature/new specs#127

Closed
hacker-DOM wants to merge 10 commits intocontracts/DX-2.0-No-Frontendfrom
contracts/feature/newSpecs
Closed

Contracts/feature/new specs#127
hacker-DOM wants to merge 10 commits intocontracts/DX-2.0-No-Frontendfrom
contracts/feature/newSpecs

Conversation

@hacker-DOM
Copy link
Copy Markdown
Contributor

Implemented everything I wanted to implement.
I'll have a look at Alex's solution for #96, other than that, we're done!

@gnosis gnosis deleted a comment from hacker-DOM Jan 11, 2018
@josojo
Copy link
Copy Markdown
Contributor

josojo commented Jan 11, 2018

approved!
Thanks for breaking the commits into clear steps. Great! Love it

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 12, 2018

Codecov Report

Merging #127 into contracts/DX-2.0-No-Frontend will increase coverage by 0.24%.
The diff coverage is 68.11%.

Impacted file tree graph

@@                       Coverage Diff                        @@
##           contracts/DX-2.0-No-Frontend     #127      +/-   ##
================================================================
+ Coverage                         54.01%   54.25%   +0.24%     
================================================================
  Files                                10       10              
  Lines                               785      763      -22     
  Branches                            107      105       -2     
================================================================
- Hits                                424      414      -10     
+ Misses                              361      349      -12
Impacted Files Coverage Δ
contracts/DutchExchange.sol 78.74% <68.11%> (+1.9%) ⬆️
contracts/Tokens/TokenTUL.sol 65.62% <0%> (-12.5%) ⬇️
contracts/Tokens/StandardToken.sol 88% <0%> (+12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 054c085...0cb025c. Read the comment docs.

Comment thread contracts/DutchExchange.sol Outdated
} else if (sellToken == ETH) {
tulipsToIssue = buyerBalance;
} else if (sellToken == ETH) {
tulipsToIssue = buyerBalance * price.num / price.den;
Copy link
Copy Markdown
Contributor

@W3stside W3stside Jan 13, 2018

Choose a reason for hiding this comment

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

should this not be tulipsToIssue = buyerBalance * price.den / price.num; as per line 580

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nope scratch that, it's good

@hacker-DOM hacker-DOM closed this Jan 15, 2018
@hacker-DOM hacker-DOM deleted the contracts/feature/newSpecs branch January 15, 2018 09:21
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.

4 participants