build(sync-ts-config): integrating the weaver packages to the monorepo build

[ruzell22]

Commit to be reviewed

---

build(sync-ts-config): integrating the weaver packages to the monorepo build

Primary Changes
---------------

1. Relocated tsconfig file to cacti-plugin-weaver-driver-fabric
2. Added comment regarding the other package not having tsconfig and will be ignoring till then

Fixes: #3366
Related to: #3069

Pull Request Requirements

• Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
• Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
• Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

• Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
• Commit Message per line must not exceed 80 characters (including spaces and special characters).

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

[petermetz]

@ruzell22 Some of the weaver CI jobs are failing. I'm guessing it's related to these changes, but not yet sure how. Could you please investigate?

[ruzell22]

@petermetz I have updated my branch and repush this PR. I can't see the weaver CI job that is failing. I have also tried to look into past latest commit in the main repo and looked for weaver that are failing and didn't see one. I would like to know the specific weaver job that is failing on your end, thank you

[outSH]

@ruzell22 I think Peter meant the following (required) CI jobs that are failing:

• Test All Docker Images Build / build_docker_fabric_driver_local (pull_request)
• Test Asset Transfer / asset-transfer-local (pull_request)
• Test Data Sharing / data-sharing-docker-local (pull_request)
• Test Data Sharing / data-sharing-local (pull_request)

Take a look at CI jobs listing on the bottom of this PR to see the details and logs why these jobs are failing

[VRamakrishna]

The folders added to the ignore list are modules and sample apps that are used in the Weaver test harnesses (see list posted by Michal above).

I'm not sure what the purpose of this PR is. Could you clarify, @ruzell22 ?

Is there a requirement for TypeScript packages to have a tsconfig.json, @petermetz ?

We are undertaking an integration of Weaver packages into the Cacti monorepo, and one of our mentees (Jennifer Bell) is addressing part of that process. The cacti-weaver-driver-fabric package was sort-of integrated into the monorepo as an experiment, whereby a shell package was created in the packages folder with a symlink to the actual source code within the weaver folder. We plan to do something similar for other packages as a stopgap measure before a more comprehensive integration (as per our envisioned architecture, see ROADMAP.md) is done, which will take a while.

But whether or not a package contains a tsconfig.json or not shouldn't matter IMO.

@ruzell22 If you are interested in exposing all other Weaver packages the way I described above for the Fabric Driver, be my guest. @sandeepnRES and I can advise you on that and you can submit a comprehensive PR then.

[VRamakrishna]

Please see my comment in the thread above. I'd recommend closing this PR and working with us on a separate PR if you are interested in carrying out integration tasks.

[petermetz]

Is there a requirement for TypeScript packages to have a tsconfig.json, @petermetz ?

@VRamakrishna It's not. Longer term, I'd love it if it was, but because I also don't want to slow down teams working on code who aren't there yet (as in not ready to have Typescript instead of Javascript).

The vision: It would be a nice bump for developer experience if the Typescript compiler was checking all NodeJS/browser source code. It makes entire classes of bugs impossible (not nearly all of them, but small wins are good too).

But whether or not a package contains a tsconfig.json or not shouldn't matter IMO.

The reason why it matters is because the automation is currently broken (it assumes that there is a tsconfig.json for all nodejs packages in the monorepo).
So we definitely need that portion of the PR that makes it so that certain directories are ignored by the script.
The part of the diff that moves around the tsconfig.json file that's in the weaver package can be removed (so that the PR only makes the change to the script under ./tools/ and then we can continue the discussion about the integration tasks in another PR/issue thread.

Please see my comment in the thread above. I'd recommend closing this PR and working with us on a separate PR if you are interested in carrying out integration tasks.

@VRamakrishna Have you seen this thread? #3366 (comment)

---

@ruzell22 Please remove the part of the diff that moves around the tsconfig.json file so that it only changes ./tools/ and that way we can simplify the review process. Please also make sure to update the ignore patterns as necessary so that the sync ts config script completes successfully.
After making those changes please re-request review from everyone to make sure.

[ruzell22]

@petermetz I have updated my repo and remove the moving of tsconfig.json. After re-running the yarn run sync-ts-config , the updated ignore patterns are the same as the one in #3367 that has been merged already. I guess this ticket and the PR can be marked as close since it will not make changes in code after the updated ignore patterns

[petermetz]

@ruzell22 On my side it still shows up in the diff containing the tsconfig.json file as being renamed. Screenshot attached below.

What I mean is that the diff should only contain the ./tools/ script.

[sandeepnRES]

Is that the only reason for this PR? one whitespace removal? I don't see any other changes in this PR.

[petermetz]

@sandeepnRES @VRamakrishna Agreed. That would actually cause a linter warning if we removed that space character so this PR is probably good to close as a no-op then.

[VRamakrishna]

@petermetz Let's close this as @ruzell22 asked for, unless you see a reason to merge it. As @sandeepnRES says above, this doesn't do anything other than change a whitespace.