hyperledger-cacti / cacti

Hyperledger Cacti is a new approach to the blockchain interoperability problem
https://wiki.hyperledger.org/display/cactus
Apache License 2.0
344 stars 286 forks source link

feat(copm): add fabric COPM implementation #3546

Closed jenniferlianne closed 2 weeks ago

jenniferlianne commented 2 months ago

Primary Change:

Secondary Changes:

Signed-off-by: Jennifer Bell jenniferlianne@gmail.com

Pull Request Requirements

Character Limit

A Must Read for Beginners For rebasing and squashing, here's a must read guide for beginners.

VRamakrishna commented 1 month ago

@jenniferlianne Can you also check the documentation (with names updated as per your discussion with @sandeepnRES) from https://jenniferlianne.github.io/cacti/references/openapi/cacti-copm-core_openapi/ into this PR? That way, when we add code to the main repo, there is (at least some) documentation explaining what it does.

(BTW....sorry for my late comments. I've still not recovered from my illness, and am taking it slow.)

jenniferlianne commented 1 month ago

@jenniferlianne Can you also check the documentation (with names updated as per your discussion with @sandeepnRES) from https://jenniferlianne.github.io/cacti/references/openapi/cacti-copm-core_openapi/ into this PR? That way, when we add code to the main repo, there is (at least some) documentation explaining what it does.

(BTW....sorry for my late comments. I've still not recovered from my illness, and am taking it slow.)

The openapi documentation should be generated under this link https://hyperledger-cacti.github.io/cacti/references/openapi/ when the code is merged. An updated preview (with the changes discussed) is here: https://jenniferlianne.github.io/cacti/references/openapi/cacti-copm-core_openapi/

VRamakrishna commented 1 month ago

@jenniferlianne Unfortunately, the project repo moved in the middle of your PR creation, so there's another change you'll need to make, though in several locations. You need to find and replace all hyperledger organization references with hyperledger-cacti references, since that's the present organization the project belongs to. This is not just about documentation or annotation, but any packages or images need to be published to the right namespace (i.e., under the present organization).

I tried to highlight each location in the code where this change needs to be made, but discovered that there are a lot of those, so stopped after a few. You'll need to do a find-and-replace throughout your code changes.

sandeepnRES commented 4 weeks ago

Looks good to me now. Thanks @jenniferlianne

VRamakrishna commented 3 weeks ago

@petermetz Can you review and unblock the merge?

petermetz commented 3 weeks ago

@petermetz Can you review and unblock the merge?

@VRamakrishna Done

@jenniferlianne Note for next time: If you don't re-request review then it doesn't pop back up on my review queue.

jenniferlianne commented 2 weeks ago

@jenniferlianne Please fix the linter errors There are no linter issues save the message about file inconsistencies prompting running yarn lint. I am unable to run yarn lint locally, it hangs. This is the error when running within the devcontainer. I have tried downgrading typescript but receive a number of error messages. Please advise.

Screenshot from 2024-11-01 15-16-09

petermetz commented 2 weeks ago

@jenniferlianne Please fix the linter errors There are no linter issues save the message about file inconsistencies prompting running yarn lint. I am unable to run yarn lint locally, it hangs. This is the error when running within the devcontainer. I have tried downgrading typescript but receive a number of error messages. Please advise.

Screenshot from 2024-11-01 15-16-09

@jenniferlianne Was it running OK before or was it always like this on your machine? What OS are you using as the host?

petermetz commented 2 weeks ago

@jenniferlianne One more data point: works fine on my machine (Ubuntu)

image

Do you want me to commit the changes directly to your branch? Then the linter should pass (since it also works fine on the CI) so that would allow us to merge it.

jenniferlianne commented 2 weeks ago

@jenniferlianne One more data point: works fine on my machine (Ubuntu)

image

Do you want me to commit the changes directly to your branch? Then the linter should pass (since it also works fine on the CI) so that would allow us to merge it.

It has never worked. I expect it's a memory issue as my machine is 16G. I am running on Ubuntu 22.04. Could you please update to complete the merge?

petermetz commented 2 weeks ago

@jenniferlianne One more data point: works fine on my machine (Ubuntu) image Do you want me to commit the changes directly to your branch? Then the linter should pass (since it also works fine on the CI) so that would allow us to merge it.

It has never worked. I expect it's a memory issue as my machine is 16G. I am running on Ubuntu 22.04. Could you please update to complete the merge?

@jenniferlianne Kk, no worries! I just force pushed to your branch, let's see if the CI passes and if it does then we are good to merge.

petermetz commented 2 weeks ago

Wohoo, thank you again @jenniferlianne !