-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
update contract docs to use compile_standard #1263
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great, @ammarqureshi! Thank you. If possible, I think it would be a good idea to doctest this so that we know if it's not working like it should. There is an example of a doctest in docs/web3.eth.account.rst
around line 84. Let me know if you have questions!
docs/contracts.rst
Outdated
"sources": { | ||
"Greeter.sol": { | ||
"content": ''' | ||
pragma solidity ^0.5.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this content
so that it is indented 4 spaces under content please?
docs/contracts.rst
Outdated
@@ -68,13 +91,13 @@ To run this example, you will need to install a few extra features: | |||
|
|||
# Create the contract instance with the newly-deployed address | |||
greeter = w3.eth.contract( | |||
address=tx_receipt.contractAddress, | |||
abi=contract_interface['abi'], | |||
address=tx_receipt.contractAddress, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be indented too please
docs/contracts.rst
Outdated
) | ||
|
||
# Display the default greeting from the contract | ||
print('Default contract greeting: {}'.format( | ||
greeter.functions.greet().call() | ||
greeter.functions.greet().call() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace :)
docs/contracts.rst
Outdated
@@ -85,9 +108,13 @@ To run this example, you will need to install a few extra features: | |||
|
|||
# Display the new greeting value | |||
print('Updated contract greeting: {}'.format( | |||
greeter.functions.greet().call() | |||
greeter.functions.greet().call() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace :)
docs/contracts.rst
Outdated
)) | ||
|
||
# When issuing a lot of reads, try this more concise reader: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 114-116 should be removed. We are no longer encouraging the use of ConciseContract in v5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my comments, @ammarqureshi! It looks like you branched off of an old version of master. It would be a good idea to pull in the most recent master. Anything that references ConciseContract should be removed. Let me know if you need help determining what should stay.
You can see the docs and run the tests from the command line by running:
make docs
.
Let me know if you need more direction!
|
||
# get abi | ||
abi = json.loads(compiled_sol['contracts']['Greeter.sol']['Greeter']['metadata'])['output']['abi'] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you're missing a few lines here:
Greeter = w3.eth.contract(abi=abi, bytecode=bytecode)
# Submit the transaction that deploys the contract
tx_hash = Greeter.constructor().transact()
# Wait for the transaction to be mined, and get the transaction receipt
tx_receipt = w3.eth.waitForTransactionReceipt(tx_hash)
docs/contracts.rst
Outdated
reader = ConciseContract(greeter) | ||
assert reader.greet() == "Nihao" | ||
|
||
.. testsetup:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be .. doctest::
so that users will be able to see it. .. testsetup::
blocks are not shown in the docs. There should be no whitespace in front of the dots.
@ammarqureshi are you still interested in completing this? |
6a68160
to
42e6389
Compare
42e6389
to
5a28130
Compare
I ran into problems installing the solidity compiler on CircleCI, so I am going to leave doctesting out for this particular example. |
What was wrong?
The contract deployment example is using
compile_source
which is no longer fully supported as mentioned in this issueHow was it fixed?
By replacing
compile_source
withcompile_standard
as mentioned by @kclowes #1245Cute Animal Picture