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.

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

[WIP] Replace ConciseContract with ContractCaller. #1034

Closed
wants to merge 1 commit into from

Conversation

Exef
Copy link
Contributor

@Exef Exef commented Aug 30, 2018

What was wrong?

Related to Issue #1025

How was it fixed?

I renamed ConciceContract to ContractReader and ConciseMethod to ReaderMethod. Also removed all unwanted methods allowance from ReaderMethod.

TODO:

  • Write documentation to the ContractCaller
  • Write test for the ContractCaller
  • Write test for deprecation message for ConciseContract and ImplicitContract

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@dylanjw
Copy link
Contributor

dylanjw commented Aug 30, 2018

The description of the work in #1025 is lacking. Rather than replacing, ConciseContract should have deprecation warnings added, and the ContractReader should be added as a new feature. After the next major version bump, ConciseContract can be deleted (v6). Going through a deprectation cycle will allow us to not break anyones code that depends on ConciseContract, and give them ample time to update before it is removed.

@Exef Im going to update the description. The ContractReader object needs some more discussion surrounding the spec that can happen in the issue #1025.

@Exef
Copy link
Contributor Author

Exef commented Aug 31, 2018

ok, I'll update this PR in that way.

@Exef Exef force-pushed the exef/1025 branch 3 times, most recently from 048c929 to 032a6da Compare September 2, 2018 15:27
@Exef Exef changed the title [WIP] Replace ConciseContract with ContractReader. [WIP] Replace ConciseContract with ContractCaller. Sep 3, 2018
@carver
Copy link
Collaborator

carver commented Sep 3, 2018

BTW, if you rebase against master, the doctest failure will go away.

@kclowes
Copy link
Collaborator

kclowes commented Dec 5, 2018

Hi @Exef! I started working on this same issue to get familiar with the library and saw this. Are you interested in completing the todos you have listed above or can I finish it up? :)

@kclowes
Copy link
Collaborator

kclowes commented Feb 21, 2019

This has been implemented in #1227. Thank you for the effort @Exef!

@kclowes kclowes closed this Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants