-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Add chaincode utilities #1095
Add chaincode utilities #1095
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.
Nice work! I had to review very carefully to find a few suggestions :-)
The auto sequencing is very nice, this really helps with iterative development. I tried it and it works seamlessly!
test-network/scripts/utils.sh
Outdated
FILE=../install-fabric.sh | ||
if [ ! -f $FILE ]; then | ||
curl -sSLO https://raw.githubusercontent.com/hyperledger/fabric/main/scripts/install-fabric.sh && chmod +x install-fabric.sh | ||
cp install-fabric.sh |
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.
I think you intended to copy it to the parent directory fabric-samples? Otherwise it doesn't work for new installs.
cp install-fabric.sh | |
cp install-fabric.sh .. |
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.
I corrected the copy command. I also added a small snippet of code to change the defaults in the config to "default" to cut down on maintenance.
test-network/network.config
Outdated
CC_VERSION="1.0.1" | ||
# -ccn | ||
# chaincode name defaults to "NA" | ||
CC_NAME="asset-transfer-basic-go" |
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.
The doc tutorials use a default name of "basic" so that any language can be used without having to change the instructions for post-deployment steps. You may want to use default name of "basic" for consistency, so that people can toggle between the existing tutorials and this new way.
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.
Corrected to "basic"
CA_IMAGETAG="1.5.7" | ||
|
||
# Using crpto vs CA. default is cryptogen | ||
CRYPTO="cryptogen" |
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 would be easier to read if there was a blank line between each property,
especially since some have the flag name and some don't (e.g. -r
below, does it go with the above property or the next property?).
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.
I moved the property to the same line as the description and added a blank line between properties.
This change is meant to help cover end user functionality that was previously covered by the blockchain VSCode Plugin. Functions added: - cc mode with package, list chaincode, invoke and query functions - auto sequencing for chaincode deployment - move variables into config file Signed-off-by: Chris Elder <[email protected]>
This change is meant to help cover end user functionality that was previously covered by the blockchain VSCode Plugin.
Functions added: