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

build(weaver-go): Update code and dependencies in Go packages for Fabric #2660

Merged
merged 5 commits into from
Sep 12, 2023
Merged

Conversation

VRamakrishna
Copy link
Contributor

@VRamakrishna VRamakrishna commented Sep 7, 2023

  • Updated Fabric Go SDK code in weaver/sdks/fabric/go-sdk to work with the latest protos and release a new version of the library.
  • Updated dependencies in go.mod for Fabric Interop Chaincode, Fabric Go CLI, and sample chaincodes.
  • Updated dependencies in relay Cargo.lock.
  • Updated Fabric testnet launch script to use the right Fabric Interop Chaincode package.
  • Set correct release versions for Fabric driver and IIN Agent local builds.

Copy link
Contributor

@jagpreetsinghsasan jagpreetsinghsasan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@VRamakrishna Please fix the commit lint check [1]. On top of that, some of my recommendations:

  1. I'd also recommend specifying the component/scope as something else than update in the commit messages
  2. I would also consider using build as the type of the commits instead of fix because users will see these in the release notes and not be able to assign any meaning to it because it's not something that was broken in the previous release and now fixed but instead it's just that the build was broken for a period of time during development in-between releases (to which the users would most likely just be noise rather than signal).
    Sorry for the nit-picking, these are just recommendations, not an actual change request because I also consider these highly subjective.

[1]
image

@VRamakrishna VRamakrishna changed the title fix(weaver-go): Update code and dependencies in Go packages for Fabric build(weaver-go): Update code and dependencies in Go packages for Fabric Sep 11, 2023
@VRamakrishna
Copy link
Contributor Author

@petermetz @sandeepnRES Fixed commits and code as per your suggestions. Please review again.

@sandeepnRES It'll need your approval as Jagpreet has added his.

Updated Fabric Go SDK library based on common protobuf changes.
Updated dependency and instructions in Weaver Go CLI for Fabric.
Updated go.mod files in components of the Weaver Fabric-interop-cc.

Signed-off-by: VRamakrishna <[email protected]>
Updated local build versions for Weaver Fabric driver and Weaver IIN Agent.
Updated Weaver relay dependencies.

Signed-off-by: VRamakrishna <[email protected]>
…ion change

Minor refactoring in Fabric testnet launch script.
Reverting unnecessary version downgrade in Weaver relay Cargo dependencies.

Signed-off-by: VRamakrishna <[email protected]>
@sandeepnRES sandeepnRES enabled auto-merge (rebase) September 12, 2023 10:07
@VRamakrishna
Copy link
Contributor Author

@petermetz cactus-cmd-api-server test is failing because of an NPM dependency failure. This has nothing to do with my code changes. Can you check?

@sandeepnRES sandeepnRES merged commit ee41066 into hyperledger-cacti:main Sep 12, 2023
107 of 132 checks passed
@petermetz
Copy link
Contributor

@petermetz cactus-cmd-api-server test is failing because of an NPM dependency failure. This has nothing to do with my code changes. Can you check?

@VRamakrishna Yeah, I traced it back to the new npm publish of v2.0.0-alpha.1 and it can't get fixed until we publish alpha.2 on npm so for now I just disabled the CI check for cmd-api-server.

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