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

test-network deployCC.sh invoke chaincode inconsistency #123

Closed
wants to merge 6 commits into from
Closed

test-network deployCC.sh invoke chaincode inconsistency #123

wants to merge 6 commits into from

Conversation

woodyjon
Copy link
Contributor

Keep consistency of the function called in the "no TLS" and "with TLS" cases

Copy link
Contributor

@nikhil550 nikhil550 left a comment

Choose a reason for hiding this comment

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

The Init call should go to the Init function. The initLedger function is a different function.

@woodyjon
Copy link
Contributor Author

But in fabcar, which is used by this script, there is no init function. The call to init (as it is in the test-network script in the "no TLS" case) does not work.

@nikhil550
Copy link
Contributor

HI @woodyjon, sorry for the delay. I asked around and it turns out I had this wrong. If you use the new fabric programing model, you can initialize the chaincode using any transaction in the chaincode. (usually this would be done with an empty instantiate or Init function added by the developer). This means that the second invoke can actually be removed. It also means that this PR can go ahead and be merged.

@woodyjon
Copy link
Contributor Author

woodyjon commented Mar 5, 2020

OK, thanks for the clarification!

@lehors
Copy link
Contributor

lehors commented Mar 20, 2020

@woodyjon Thanks for your contribution but we won't be able to merge it without a sign-off. Can you please amend your commit with a sign-off in the log? See https://hyperledger-fabric.readthedocs.io/en/master/CONTRIBUTING.html#legal-stuff

@woodyjon
Copy link
Contributor Author

I just added the sign-off message to the commits. Is it ok like so?

@lehors
Copy link
Contributor

lehors commented Mar 23, 2020

The DCO check is still failing. Please, follow the instructions:
https://github.com/hyperledger/fabric-samples/pull/123/checks?check_run_id=526791523
Thanks.

nikhil550 and others added 6 commits March 23, 2020 09:20
Signed-off-by: NIKHIL E GUPTA <negupta@us.ibm.com>
Signed-off-by: Jon <woodyjon@gmail.com>
Change 2.0.0-beta to 2.0.0 when CommercialPaper uses the test network
Add test network to the azure pipelines
Correct test network envvar script

Signed-off-by: Matthew B White <whitemat@uk.ibm.com>
Signed-off-by: Jon <woodyjon@gmail.com>
Signed-off-by: Jonathan Gross <woodyjon@gmail.com>
Keep consistency of the function called in the "no TLS" and "with TLS" cases

Signed-off-by: Jon <woodyjon@gmail.com>
Signed-off-by: Jonathan Gross <woodyjon@gmail.com>
…ct one

It was actually the other way around, it is the no TLS line which was wrong and should have ready `initLedger` instead of `init`.

Signed-off-by: Jon <woodyjon@gmail.com>
Signed-off-by: Jonathan Gross <woodyjon@gmail.com>
Keep consistency of the function called in the "no TLS" and "with TLS" cases

Signed-off-by: Jonathan Gross <woodyjon@gmail.com>
Signed-off-by: Jon <woodyjon@gmail.com>
Signed-off-by: Jonathan Gross <woodyjon@gmail.com>
…ct one

It was actually the other way around, it is the no TLS line which was wrong and should have ready `initLedger` instead of `init`.

Signed-off-by: Jonathan Gross <woodyjon@gmail.com>
Signed-off-by: Jon <woodyjon@gmail.com>
Signed-off-by: Jonathan Gross <woodyjon@gmail.com>
@woodyjon
Copy link
Contributor Author

There you go 😀

@woodyjon
Copy link
Contributor Author

If needed, I can redo the PR more cleanly, with a single commit.

@lehors
Copy link
Contributor

lehors commented Mar 23, 2020

No worries, we'll squash the commits. Thanks.

@lehors
Copy link
Contributor

lehors commented Mar 23, 2020

Wait, now there are 87 files changed... something went wrong.

@woodyjon
Copy link
Contributor Author

Do I make a new PR?

@lehors
Copy link
Contributor

lehors commented Mar 27, 2020

If that's easier for you then just do that. What's clear is that we can't merge this PR as it stands so something needs to be done.

@woodyjon
Copy link
Contributor Author

I close this PR because I messed it up. Cleaner PR is there: #141

@woodyjon woodyjon closed this Mar 27, 2020
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