refactor: dangling package.json inside packages directory #682

[petermetz]

Dependencies

Depends on #656

Commit to review

Changes

1. Moved the dangling folders under the package that was created
2. Fixed linter errors (1000+)
3. Also migrated the cartrade example to import via the package

TODO:

1. Other examples to be converted into packages that are part of the build
2. Examples made to work with the build system as well

Fixes #682

Signed-off-by: Peter Somogyvari [email protected]

[kikoncuo]

What's the new package doing? needs-a-name

[petermetz]

What's the new package doing? needs-a-name

@kikoncuo Yeah, wasn't sure what to name it even as a suggestion/draft because while trying to come up with something a few questions came up that would need to be answered first. We can hash it out in the upcoming maintainers meeting if all goes well.

cc: @takeutak @sfuji822

[github-actions]

🎉 Great news! Looks like all the dependencies have been resolved:

• fix(api-client): default consortium provider timeout test flake #656

💡 To add or remove a dependency please update this issue/PR description.

Brought to you by Dependent Issues (:robot: ). Happy coding!

[petermetz]

@sfuji822 @takeutak Kindly requesting that you share some initial thoughts on the refactoring proposed here.
Just talking about the direction of the refactor not the entirety of the changes because there's hundreds of those and they also need to be rebased/further improved (but I don't want to invest more effort into that without having consensus).
Thank you very much in advance!

[takeutak]

@petermetz Sorry for the delay in my comments.
First, regarding the refactoring proposed in this PR, I think that unification of the notation of variable names is necessary in the future. However, I would like to check the operation of LedgerPlugin before changing it. Could you please exclude the following directory from this PR?

• examples/cartrade, electricity-trade, run-transaction, test-run-transaction
• packages/business-logic-plugin, config, ledger-plugin, routing-interface, copyStaticAssets.ts, jest.config.js, package.json, tsconfig.json, tslint.json, script-build-packages.sh

[petermetz]

@takeutak No worries. I understand everyone's very busy.

However, I would like to check the operation of LedgerPlugin before changing it.

That's fine. Should we revisit next week in that case? Do you think your check will have finished by then?
It's much easier for me to not exclude and then re-include everything the PR does/changes again, instead I can just wait a bit for your check to finish.

[takeutak]

With this change, files such as the files are deleted. I will ask you to modify the PR so that it does not reflect the file change without deleting the file.

• examples/cartrade, electricity-trade, run-transaction, test-run-transaction
• packages/business-logic-plugin, config, ledger-plugin, routing-interface, copyStaticAssets.ts, jest.config.js, package.json, tsconfig.json, tslint.json, script-build-packages.sh

[petermetz]

@takeutak Files weren't really deleted just moved, it was showing them as deleted because I also spent a few hours fixing linter errors on them and those changing + the move made git think that the files were deleted.
I started from scratch with the whole thing again and this time I did not do any linter fixes so the files are shown as having been moved not deleted. Does this look better? What do you think of the refactoring approach in general?

[petermetz]

@takeutak +1

[takeutak]

@petermetz Sorry, I overlooked this PR since its status was change-request. I changed the config of GitHub notification and I will try to avoid these overlooking.
I commented on "./packages/jest.config.js".

[takeutak]

@petermetz I misunderstood your proposal on this pull-request. It seems that our proposal is to move socket.io typed validators codes of the following codes to some subdirectory

• examples/cartrade, electricity-trade, run-transaction, test-run-transaction
• packages/business-logic-plugin, config, ledger-plugin, routing-interface, copyStaticAssets.ts, jest.config.js, package.json, tsconfig.json, tslint.json, script-build-packages.sh
  but I cannot agree with your proposal because this proposal cannot take advantages of both types of openAPI-typed Validators and socket.io-typed validators.

I think your proposal means that duplicated package.json are inconvenient for CI checking system. But I'd like to keep the above codes on this repository in as easy-to-use form as the other codes. Would you mind if you give me some advice on this issues?

[codecov-commenter]

Codecov Report

Merging #719 (fd46be7) into main (a8e21a9) will not change coverage.
The diff coverage is n/a.

❗ Current head fd46be7 differs from pull request most recent head 0d79b9b. Consider uploading reports for the commit 0d79b9b to get more accurate results

@@           Coverage Diff           @@
##             main     #719   +/-   ##
=======================================
  Coverage   71.49%   71.49%           
=======================================
  Files         298      298           
  Lines       10743    10743           
  Branches     1321     1321           
=======================================
  Hits         7681     7681           
  Misses       2364     2364           
  Partials      698      698           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8e21a9...0d79b9b. Read the comment docs.

[takeutak]

@petermetz Thanks for contributing, but I cannot agree with your refactoring plan because some example applications (car-trade etc.) cannot work well on your suggested directory structure.
Instead of this, in the future I have a plan to move the codes as the following structure:

cactus-plugin-ledger-connector-{go-ethereum,sawtooth, ...} // moved from ledger-plugin/validator
  src/main
       /test
  package.json
  tsconfig.json
cactus-cmd-socketio-server // moved from routing-interface, ledger-plugin/verifier, etc.
  src/main
       /test
  package.json
  tsconfig.json

Then I have a favor to ask you. Could you put the code back in this pull-request? I would like to refactor the codes on another pull-request by myself to ensure it works.

[petermetz]

some example applications (car-trade etc.) cannot work well

@takeutak By the car-trade example not working well, do you mean that A) it doesn't work at all, or B) that it works, just not well enough?

If A could you please post error logs, stack traces and how you got to them (what commands you ran etc.)
If B could you please highlight the specific issues that make it not work well enough?

[takeutak]

@petermetz I meant that "I'd like to refactor the codes by myself while keeping business-logic applications working properly."

As I explained at the last contributor meeting, I'm working to present a revised plan for refactoring directories later as the following structure. Could you wait until then?

cactus-plugin-ledger-connector-{go-ethereum,sawtooth, ...} // moved from ledger-plugin/validator
  src/main
       /test
  package.json
  tsconfig.json
cactus-cmd-socketio-server // moved from routing-interface, ledger-plugin/verifier, etc.
  src/main
       /test
  package.json
  tsconfig.json

[petermetz]

@takeutak That's the thing. The car trade should work and if you are happy with that I shall put in even more effort to migrate over the rest of the business logic code examples that you have. Could you verify if the car trade works on this PR and if it doesn't then we could debug it together in a pair prog. session. And if it's not working because there's a bug, then I can fix it based on that. (It did work on my machine when I was testing, but I could only partially verify it due to that issue with the Fabric peer container crashing - I emailed you the crash log the other week IIRC)

[takeutak]

@petermetz Sorry for late answering.
As I'm investigating now, but I found that there are some bugs on script-start-docker.sh (not car-trade app itself) of fabric container including fabcar chaincode.
https://github.com/hyperledger/cactus/tree/main/tools/docker/fabric14-fabcar-testnet

In some days ago, the script can be executed successfully, but not it is not executed due to some reasons (maybe the wallet key problem of fabcar) I will fix the problem in a few days, so could you wait a moment?

[petermetz]

@takeutak No worries, let's talk once the fix is available. Thank you very much in advance for the help!

[takeutak]

@petermetz I edited the following structures in order to arrange some files to your directory style. Would you review this PR again? If you agree, I'd like to merge these codes to main.

- etc/cactus: (config files)
- examples:
  - cartrade
  - discounted-cartrade
  - electricity-trade
- packages:
  - cactus-cmd-socketio-server
  - cactus-plugin-ledger-connector-go-ethereum-socketio
  - cactus-plugin-ledger-connector-fabric-socketio
  - cactus-plugin-ledger-connector-sawtooth-socketio

[takeutak]

@petermetz Could you resolve the conflicts on the following files?

• /.cspell.json
• /.eslintignore
• /package.json
• /yarn.lock