search

hyperledger / cacti

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

build(weaver): eliminate need for package-local.json manifests #2601

Open petermetz opened 1 year ago

petermetz commented 1 year ago

Description

VSCode does not recognize package-local.json files semantically as package.json manifests meaning that the convenience functionalities it offers for editing package.json files are turned off (and I haven't figured out a way to enable them other than renaming the files).

My understanding (just from reading the code and inferring things) is that the only reason we needed these local vs remote package.json files is because there was no monorepo in place in the old weaver codebase and so this copying local and remote package.json files was the way symlinking local code got implemented.

Now that the weaver code is part of the monorepo build we can do away with these mechanisms which would simplify the build scripts a lot as well because

  1. The shell scripts that are doing the package.json copy/move operations to shuffle around local and remote versions can be deleted entirely
  2. yarn install can be ran from any directory of the monorepo and it will "just work"
  3. npm ci calls can be replaced with yarn install --production --frozen-lockfile

Acceptance Criteria

  1. All package-local.json files were deleted
  2. All build scripts updated to delete the logic that copies/moves around package.json files at build time
  3. The package.json files that are getting used can always be looked at just based on what's stored in version control instead of having to guess what changes are being made to the files by the build at runtime.

cc: @VRamakrishna @sandeepnRES

krisstern commented 5 months ago

Hi, I am interested in working on this issue

petermetz commented 5 months ago

@krisstern Got it, assigned to you just now! Please try to keep the pull request size to a minimum to make it easy to test/review it. What I recommend is that you pick a single package first and apply the change there and then submit a PR with it just so that we can start the conversation whether it's agreeable by all the maintainers. This way you avoid doing a large chunk of work upfront and risking that big refactors will be needed on it. Long story short: Let's establish the pattern of the solution first and then we can apply it project-wide.

Thank you in advance!

krisstern commented 5 months ago

Sure @petermetz! I will keep that in mind moving forward. So I will submit a series of PR's (instead of having one big one) for this issue and complete the task incrementally.

krisstern commented 4 months ago

Hi @petermetz, may I ask what are the origins of the "package-local.json" files if you happen to know the context? I have been trying to find out if it is due to a particular setup, but could not find much relevant info.

outSH commented 4 months ago

Hi @petermetz, may I ask what are the origins of the "package-local.json" files if you happen to know the context? I have been trying to find out if it is due to a particular setup, but could not find much relevant info.

@sandeepnRES @VRamakrishna could you clarify origins of the package-local.json if you find a moment? :)

petermetz commented 4 months ago

Hi @petermetz, may I ask what are the origins of the "package-local.json" files if you happen to know the context? I have been trying to find out if it is due to a particular setup, but could not find much relevant info.

@sandeepnRES @VRamakrishna could you clarify origins of the package-local.json if you find a moment? :)

@outSH: They didn't have the workspaces feature enabled in the old Hyperledger Labs repo where weaver was living back in the day and without the npm/yarn workspace feature doing the linking, the only way you could make it link the locally developed changes across packages together was to declare the dependency with the "file://some-path-to-the-package" (the stuff you see in the*-local.json` files). So it had a purpose back then to test your local changes in packages that were dependencies of some other package. But now with the yarn workspaces + lerna-lite build system we don't need them (except the CI scripts still depend on them being that way but that's something that could be eliminated)