Skip to content
This repository was archived by the owner on Jul 9, 2021. It is now read-only.

Conversation

fabioberger
Copy link
Contributor

@fabioberger fabioberger commented Nov 7, 2019

Description

This PR:

  • Removes ZRXToken, DutchAuction and all assetProxy contracts from abi-gen-wrappers and contract-wrapper packages in an effort to make these packages smaller.
  • As part of refactoring the above, I needed to move ExchangeTransferSimulator tests out of order-utils in order to have the tests continue to refer to contract-* wrappers no longer available via abi-gen-wrappers without importing a bunch of wrappers into order-utils. We wanted to move this and it's deps anyways into contracts-exchange since that is the only place that still uses this functionality and we want to re-direct external devs to use DevUtils wrapper methods for order validation.
  • Stopped deploying the old DutchAuction contract in the migrations script. It can be re-added once it's been updated.

Testing instructions

Types of changes

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@buildsize
Copy link

buildsize bot commented Nov 7, 2019

File name Previous Size New Size Change
init.py 26.91 KB 26.91 KB 0 bytes (0%)
abi_gen_dummy.ts 184.89 KB [deleted]
lib_dummy.ts 5.23 KB [deleted]
test_lib_dummy.ts 14.8 KB [deleted]
environment.pickle 1.61 MB 1.61 MB 0 bytes (0%)
index.doctree 194.53 KB 194.53 KB 0 bytes (0%)
.buildinfo 230 bytes 230 bytes 0 bytes (0%)
genindex.html 5.6 KB 5.6 KB 0 bytes (0%)
index.html 2.52 KB 2.52 KB 0 bytes (0%)
objects.inv 375 bytes 375 bytes 0 bytes (0%)
py-modindex.html 3.07 KB 3.07 KB 0 bytes (0%)
search.html 2.84 KB 2.84 KB 0 bytes (0%)
searchindex.js 6.3 KB 6.3 KB 0 bytes (0%)
index.rst.txt 415 bytes 415 bytes 0 bytes (0%)
alabaster.css 10.92 KB 10.92 KB 0 bytes (0%)
basic.css 11.89 KB 11.89 KB 0 bytes (0%)
custom.css 42 bytes 42 bytes 0 bytes (0%)
doctools.js 9.05 KB 9.05 KB 0 bytes (0%)
documentation_options.js 303 bytes 303 bytes 0 bytes (0%)
file.png 286 bytes 286 bytes 0 bytes (0%)
jquery-[version].js 273.79 KB 273.79 KB 0 bytes (0%)
jquery.js 86.08 KB 86.08 KB 0 bytes (0%)
language_data.js 10.59 KB 10.59 KB 0 bytes (0%)
minus.png 90 bytes 90 bytes 0 bytes (0%)
plus.png 90 bytes 90 bytes 0 bytes (0%)
pygments.css 4.69 KB 4.69 KB 0 bytes (0%)
searchtools.js 15.61 KB 15.61 KB 0 bytes (0%)
underscore-[version].js 34.34 KB 34.34 KB 0 bytes (0%)
underscore.js 11.86 KB 11.86 KB 0 bytes (0%)
contract_addresses.html 16.8 KB 16.8 KB 0 bytes (0%)
contract_artifacts.html 8.24 KB 8.24 KB 0 bytes (0%)
json_schemas.html 12.55 KB 12.55 KB 0 bytes (0%)
order_utils.html 46.85 KB 46.85 KB 0 bytes (0%)
erc20_token.html 93.31 KB 93.31 KB 0 bytes (0%)
exchange.html 678.51 KB 678.51 KB 0 bytes (0%)
tx_params.html 9.41 KB 9.41 KB 0 bytes (0%)
local_message_signer.html 15.07 KB 15.07 KB 0 bytes (0%)
asset_data_utils.html 22.65 KB 22.65 KB 0 bytes (0%)
default_api.html 118.19 KB 118.19 KB 0 bytes (0%)
asset_proxy_owner.html 350.44 KB 350.44 KB 0 bytes (0%)
coordinator.html 133.77 KB 133.77 KB 0 bytes (0%)
coordinator_registry.html 39.72 KB 39.72 KB 0 bytes (0%)
dutch_auction.html 59.6 KB 59.6 KB 0 bytes (0%)
erc20_proxy.html 111 KB 111 KB 0 bytes (0%)
erc721_proxy.html 111.12 KB 111.12 KB 0 bytes (0%)
erc721_token.html 148.29 KB 148.29 KB 0 bytes (0%)
eth_balance_checker.html 24.65 KB [deleted]
forwarder.html 110.46 KB 110.46 KB 0 bytes (0%)
i_asset_proxy.html 39.32 KB 39.32 KB 0 bytes (0%)
i_validator.html 30.37 KB 30.37 KB 0 bytes (0%)
i_wallet.html 27.63 KB 27.63 KB 0 bytes (0%)
multi_asset_proxy.html 148.59 KB 148.59 KB 0 bytes (0%)
order_validator.html 120.52 KB 120.52 KB 0 bytes (0%)
weth9.html 133.98 KB 133.98 KB 0 bytes (0%)
zrx_token.html 111.93 KB 111.93 KB 0 bytes (0%)
dev_utils.html 580.48 KB 580.48 KB 0 bytes (0%)
types.html 8.68 KB 8.68 KB 0 bytes (0%)
erc1155_mintable.html 288.23 KB 288.23 KB 0 bytes (0%)
erc1155_proxy.html 129.7 KB 129.7 KB 0 bytes (0%)
static_call_proxy.html 39.44 KB 39.44 KB 0 bytes (0%)

@coveralls
Copy link

coveralls commented Nov 7, 2019

Coverage Status

Coverage remained the same at 75.286% when pulling 5c40992 on refactor/removeUnusedContractWrappers into 869d2c0 on development.

@fabioberger fabioberger marked this pull request as ready for review November 8, 2019 11:49
@fabioberger fabioberger requested a review from xianny November 8, 2019 11:49
Copy link
Contributor

@xianny xianny left a comment

Choose a reason for hiding this comment

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

One question about instantiating ExchangeTransferSimulator with an instance of DevUtilsContract instead of a provider.

Everything else looks good to me! Thanks for updating exchange_transfer_simulator_test.ts with the blockchainTests pattern.

Copy link
Contributor

@xianny xianny left a comment

Choose a reason for hiding this comment

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

approved after CI passes!

@fabioberger fabioberger requested a review from albrow as a code owner November 11, 2019 12:00
@fabioberger fabioberger force-pushed the refactor/removeUnusedContractWrappers branch from 258a2a4 to 90640a4 Compare November 11, 2019 17:53
@fabioberger fabioberger merged commit 35099d9 into development Nov 11, 2019
@fabioberger fabioberger deleted the refactor/removeUnusedContractWrappers branch November 11, 2019 20:59
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