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

Add design document for fpc with cc-tools #773

Conversation

osamamagdy
Copy link
Contributor

This PR adds the design document and how we approach the integration project between CC-Tools and FPC

@osamamagdy osamamagdy requested a review from a team as a code owner September 27, 2024 21:29
@osamamagdy osamamagdy force-pushed the feat/fpc-with-cc-tools-design-document branch 4 times, most recently from 6bab6b2 to 19a889b Compare September 27, 2024 21:39
Copy link
Contributor

@mbrandenburger mbrandenburger left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this great proposal! I highly support this work!

Please see a few cosmetic comments.

docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
@mbrandenburger mbrandenburger added feature documentation This contains documentation/description/comments labels Oct 1, 2024
@mbrandenburger mbrandenburger self-assigned this Oct 1, 2024
@munapower
Copy link
Contributor

I have looked at the design document. These are my comments:

  • please review sintax. There are a lot of sentences that are missing the period at the end.
  • review code snippets that the indentations appear so it is easier to read. For example it should be like this:
func (f *FpcStubInterface) PutState(key string, value []byte) error {
    encValue, err := f.sep.EncryptState(value)
    if err != nil {
         return err
    }
    return f.PutPublicState(key, encValue)
 }

-In this sentence

For this, It's better to inject the fpc stub wrapper in the middle between fabric stub and cc-tools stub wrapper 2.

What does that 2 mean?

@osamamagdy osamamagdy force-pushed the feat/fpc-with-cc-tools-design-document branch 2 times, most recently from f9dfe23 to 2ce4558 Compare October 11, 2024 01:55
@osamamagdy osamamagdy force-pushed the feat/fpc-with-cc-tools-design-document branch from 2ce4558 to cdf1eca Compare October 11, 2024 01:56
@osamamagdy osamamagdy force-pushed the feat/fpc-with-cc-tools-design-document branch from 720db21 to 51b6004 Compare October 11, 2024 11:07
@osamamagdy osamamagdy force-pushed the feat/fpc-with-cc-tools-design-document branch 2 times, most recently from 46cdeb0 to 735676d Compare October 11, 2024 13:51
Signed-off-by: osamamagdy <[email protected]>
@osamamagdy osamamagdy force-pushed the feat/fpc-with-cc-tools-design-document branch from 735676d to 3f75074 Compare October 11, 2024 13:51
Copy link
Contributor

@mbrandenburger mbrandenburger left a comment

Choose a reason for hiding this comment

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

LGTM

docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
docs/design/integrate-with-cc-tools/README.md Outdated Show resolved Hide resolved
@osamamagdy osamamagdy force-pushed the feat/fpc-with-cc-tools-design-document branch from 90f5433 to 1474dfa Compare October 17, 2024 20:25
Signed-off-by: osamamagdy <[email protected]>
@osamamagdy osamamagdy force-pushed the feat/fpc-with-cc-tools-design-document branch from 1474dfa to 4a5744e Compare October 17, 2024 20:59
@mbrandenburger mbrandenburger merged commit 0af7194 into hyperledger:main Oct 19, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This contains documentation/description/comments feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants