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

fix(deps): bulk add missing dependencies - 2023-11-02 #2859

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

petermetz
Copy link
Contributor

  1. Added missing dependencies everywhere to all the packages
  2. The exception to the above is test runners and related dependencies.
    You can see the detailed exclusion list in the source code of the script
    itself [1]. We should make these configurable later on as well. For
    reference, this is what the exclusions were declared as when I ran the
    tool in order to then proceed to update the package.json files:
ignorePatterns: [
    // files matching these patterns will be ignored
    "sandbox",
    "dist",
    "bower_components",
    "node_modules"
],
ignoreMatches: [
    // ignore dependencies that matches these globs
    "grunt-*",
    "jest-extended",
    "tape-promise",
    "tape",
    "tap",
    "@ionic-native/*"
],
  1. There were instances of missing dependency usages where we did NOT
    have to add the dependencies to package.json files because what we could
    do instead is just import types at development time by using the
    import type { .. } from "..."; syntax of Typescript which means that
    the import is disappeared during transpilation completely. So this is why
    some source code files were also modified and not strictly just package.json
    files.
  2. Added a script in the ./tools/custom-check directory to audit the
    entire mono-repo for missing NodeJS dependencies.

We've had at least a dozen packages that were missing production
dependency declarations from their package.json files. The usual suspects
here are packages that are contained by the root node_modules folder
which masks the problem at development time (e.g. the compiler won't
let you know about the missing dependencies).

For a future-proof solution we should add a commit hook or other validation
that runs the custom check that I added [2] here to verify that there are no
missing dependencies in the project

[1] ./tools/custom-checks/check-missing-node-deps.ts
[2] $ yarn tools:check-missing-node-deps

Fixes #2857

Signed-off-by: Peter Somogyvari peter.somogyvari@accenture.com

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 petermetz enabled auto-merge (rebase) November 2, 2023 21:19
@petermetz petermetz force-pushed the petermetz/issue2857 branch 2 times, most recently from 99715b9 to c0b50d6 Compare November 2, 2023 23:04
outSH added a commit to outSH/cactus that referenced this pull request Nov 3, 2023
- Refactor test ledger `indy-testnet` into `indy-all-in-one`. New package uses
    the latest indy version, has healtcheck script, updated startup / cleanup
    scripts.
- Remove `indy-sdk-cli` image since it's not used anymore.
- Refactor `cactus-example-discounted-asset-trade` to use own aries agent
    instead of indy connector. This way it doesn't need to use indy-sdk anymore,
    and python indy connector can be safely removed / upgraded.
- Update sample app readme to explain current workflow.
- Remove client scripts since `cactus-example-discounted-asset-trade-client`
    can now be used to interact with the sample app.
- Add `cactus-example-discounted-asset-trade-client`. It contains script for
    setting up test credentials on the ledger, script with interactive menu for
    interacting with `cactus-example-discounted-asset-trade` sample app,
    and bunch of helper functions used for writing these apps.

Depends on hyperledger-cacti#2859
Depends on hyperledger-cacti#2860

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
@petermetz petermetz force-pushed the petermetz/issue2857 branch from c0b50d6 to a7892b4 Compare November 7, 2023 00:07
@petermetz petermetz requested a review from outSH November 7, 2023 16:45
@outSH
Copy link
Contributor

outSH commented Nov 9, 2023

@petermetz Can't give approval on this (or any other PR) because I'm missing permissions to this repo, I think Sato-san mentioned it on discord. Could you look into that in spare moment and give me write access so I can perform the reviews?

@petermetz
Copy link
Contributor Author

@petermetz Can't give approval on this (or any other PR) because I'm missing permissions to this repo, I think Sato-san mentioned it on discord. Could you look into that in spare moment and give me write access so I can perform the reviews?

@outSH Oops, sorry, we need to add you to the Hyperledger GitHub org and then I can grant you permissions. I can't add you to the GH org myself but I sent in a request to the HL staff to get it done here: hyperledger/governance#177
You should see an email in your inbox soon with an invitation to join the Hyperledger GitHub org and once you accept through the link in there then I can fix the permissions!

@petermetz
Copy link
Contributor Author

@outSH You should have full permissions now on the repo (admin rights)

Copy link
Contributor

@outSH outSH left a comment

Choose a reason for hiding this comment

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

@petermetz Thank you!

LGTM

1. Added missing dependencies everywhere to all the packages
2. The exception to the above is test runners and related dependencies.
You can see the detailed exclusion list in the source code of the script
itself [1]. We should make these configurable later on as well. For
reference, this is what the exclusions were declared as when I ran the
tool in order to then proceed to update the package.json files:

```typescript
ignorePatterns: [
    // files matching these patterns will be ignored
    "sandbox",
    "dist",
    "bower_components",
    "node_modules"
],
ignoreMatches: [
    // ignore dependencies that matches these globs
    "grunt-*",
    "jest-extended",
    "tape-promise",
    "tape",
    "tap",
    "@ionic-native/*"
],
```
3. There were instances of missing dependency usages where we did NOT
have to add the dependencies to package.json files because what we could
do instead is just import types at development time by using the
`import type { .. } from "...";`  syntax of Typescript which means that
the import is disappeared during transpilation completely. So this is why
some source code files were also modified and not strictly just package.json
files.
4. One exception to the above is the google-sm-keychain plugin's mock
code where minimal code alternations were necessary to satisfy the compiler.
With that said, no behavioral code change was done here either, just the
elimination of some redundant assignments.
5. Added a script in the ./tools/custom-check directory to audit the
entire mono-repo for missing NodeJS dependencies.

We've had at least a dozen packages that were missing production
dependency declarations from their package.json files. The usual suspects
here are packages that are contained by the root node_modules folder
which masks the problem at development time (e.g. the compiler won't
let you know about the missing dependencies).

For a future-proof solution we should add a commit hook or other validation
that runs the custom check that I added [2] here to verify that there are no
missing dependencies in the project

[1] `./tools/custom-checks/check-missing-node-deps.ts`
[2] `$ yarn tools:check-missing-node-deps`

Fixes hyperledger-cacti#2857

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@petermetz petermetz merged commit 8addb01 into hyperledger-cacti:main Nov 14, 2023
38 of 61 checks passed
@petermetz petermetz deleted the petermetz/issue2857 branch November 14, 2023 18:11
outSH added a commit to outSH/cactus that referenced this pull request Dec 12, 2023
- Refactor test ledger `indy-testnet` into `indy-all-in-one`. New package uses
    the latest indy version, has healtcheck script, updated startup / cleanup
    scripts.
- Remove `indy-sdk-cli` image since it's not used anymore.
- Refactor `cactus-example-discounted-asset-trade` to use own aries agent
    instead of indy connector. This way it doesn't need to use indy-sdk anymore,
    and python indy connector can be safely removed / upgraded.
- Update sample app readme to explain current workflow.
- Remove client scripts since `cactus-example-discounted-asset-trade-client`
    can now be used to interact with the sample app.
- Add `cactus-example-discounted-asset-trade-client`. It contains script for
    setting up test credentials on the ledger, script with interactive menu for
    interacting with `cactus-example-discounted-asset-trade` sample app,
    and bunch of helper functions used for writing these apps.

Depends on hyperledger-cacti#2859
Depends on hyperledger-cacti#2860

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
outSH added a commit to outSH/cactus that referenced this pull request Dec 13, 2023
- Refactor test ledger `indy-testnet` into `indy-all-in-one`. New package uses
    the latest indy version, has healtcheck script, updated startup / cleanup
    scripts.
- Remove `indy-sdk-cli` image since it's not used anymore.
- Refactor `cactus-example-discounted-asset-trade` to use own aries agent
    instead of indy connector. This way it doesn't need to use indy-sdk anymore,
    and python indy connector can be safely removed / upgraded.
- Update sample app readme to explain current workflow.
- Remove client scripts since `cactus-example-discounted-asset-trade-client`
    can now be used to interact with the sample app.
- Add `cactus-example-discounted-asset-trade-client`. It contains script for
    setting up test credentials on the ledger, script with interactive menu for
    interacting with `cactus-example-discounted-asset-trade` sample app,
    and bunch of helper functions used for writing these apps.

Depends on hyperledger-cacti#2859
Depends on hyperledger-cacti#2860

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
outSH added a commit to outSH/cactus that referenced this pull request Jan 19, 2024
- Refactor test ledger `indy-testnet` into `indy-all-in-one`. New package uses
    the latest indy version, has healtcheck script, updated startup / cleanup
    scripts.
- Remove `indy-sdk-cli` image since it's not used anymore.
- Refactor `cactus-example-discounted-asset-trade` to use own aries agent
    instead of indy connector. This way it doesn't need to use indy-sdk anymore,
    and python indy connector can be safely removed / upgraded.
- Update sample app readme to explain current workflow.
- Remove client scripts since `cactus-example-discounted-asset-trade-client`
    can now be used to interact with the sample app.
- Add `cactus-example-discounted-asset-trade-client`. It contains script for
    setting up test credentials on the ledger, script with interactive menu for
    interacting with `cactus-example-discounted-asset-trade` sample app,
    and bunch of helper functions used for writing these apps.

Depends on hyperledger-cacti#2859
Depends on hyperledger-cacti#2860

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
outSH added a commit to outSH/cactus that referenced this pull request Jan 19, 2024
- Refactor test ledger `indy-testnet` into `indy-all-in-one`. New package uses
    the latest indy version, has healtcheck script, updated startup / cleanup
    scripts.
- Remove `indy-sdk-cli` image since it's not used anymore.
- Refactor `cactus-example-discounted-asset-trade` to use own aries agent
    instead of indy connector. This way it doesn't need to use indy-sdk anymore,
    and python indy connector can be safely removed / upgraded.
- Update sample app readme to explain current workflow.
- Remove client scripts since `cactus-example-discounted-asset-trade-client`
    can now be used to interact with the sample app.
- Add `cactus-example-discounted-asset-trade-client`. It contains script for
    setting up test credentials on the ledger, script with interactive menu for
    interacting with `cactus-example-discounted-asset-trade` sample app,
    and bunch of helper functions used for writing these apps.

Depends on hyperledger-cacti#2859
Depends on hyperledger-cacti#2860

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
outSH added a commit to outSH/cactus that referenced this pull request Jan 19, 2024
- Refactor test ledger `indy-testnet` into `indy-all-in-one`. New package uses
    the latest indy version, has healtcheck script, updated startup / cleanup
    scripts.
- Remove `indy-sdk-cli` image since it's not used anymore.
- Refactor `cactus-example-discounted-asset-trade` to use own aries agent
    instead of indy connector. This way it doesn't need to use indy-sdk anymore,
    and python indy connector can be safely removed / upgraded.
- Update sample app readme to explain current workflow.
- Remove client scripts since `cactus-example-discounted-asset-trade-client`
    can now be used to interact with the sample app.
- Add `cactus-example-discounted-asset-trade-client`. It contains script for
    setting up test credentials on the ledger, script with interactive menu for
    interacting with `cactus-example-discounted-asset-trade` sample app,
    and bunch of helper functions used for writing these apps.

Depends on hyperledger-cacti#2859
Depends on hyperledger-cacti#2860

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
outSH added a commit to outSH/cactus that referenced this pull request Jan 19, 2024
- Refactor test ledger `indy-testnet` into `indy-all-in-one`. New package uses
    the latest indy version, has healtcheck script, updated startup / cleanup
    scripts.
- Remove `indy-sdk-cli` image since it's not used anymore.
- Refactor `cactus-example-discounted-asset-trade` to use own aries agent
    instead of indy connector. This way it doesn't need to use indy-sdk anymore,
    and python indy connector can be safely removed / upgraded.
- Update sample app readme to explain current workflow.
- Remove client scripts since `cactus-example-discounted-asset-trade-client`
    can now be used to interact with the sample app.
- Add `cactus-example-discounted-asset-trade-client`. It contains script for
    setting up test credentials on the ledger, script with interactive menu for
    interacting with `cactus-example-discounted-asset-trade` sample app,
    and bunch of helper functions used for writing these apps.

Depends on hyperledger-cacti#2859
Depends on hyperledger-cacti#2860

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
outSH added a commit that referenced this pull request Jan 19, 2024
- Refactor test ledger `indy-testnet` into `indy-all-in-one`. New package uses
    the latest indy version, has healtcheck script, updated startup / cleanup
    scripts.
- Remove `indy-sdk-cli` image since it's not used anymore.
- Refactor `cactus-example-discounted-asset-trade` to use own aries agent
    instead of indy connector. This way it doesn't need to use indy-sdk anymore,
    and python indy connector can be safely removed / upgraded.
- Update sample app readme to explain current workflow.
- Remove client scripts since `cactus-example-discounted-asset-trade-client`
    can now be used to interact with the sample app.
- Add `cactus-example-discounted-asset-trade-client`. It contains script for
    setting up test credentials on the ledger, script with interactive menu for
    interacting with `cactus-example-discounted-asset-trade` sample app,
    and bunch of helper functions used for writing these apps.

Depends on #2859
Depends on #2860

Signed-off-by: Michal Bajer <michal.bajer@fujitsu.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix(deps): bulk add missing dependencies - 2023-11-02
3 participants