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

chore: [M3-8786] - Update and clean up @types/node package #11157

Conversation

bnussman-akamai
Copy link
Member

@bnussman-akamai bnussman-akamai commented Oct 24, 2024

Description 📝

How to test 🧪

  • Verify all type-checking passes ✅
  • Check for regressions in files that were touched 👁️

As an Author I have considered 🤔

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@bnussman-akamai bnussman-akamai added the Dependencies Pull requests that update a dependency file label Oct 24, 2024
@bnussman-akamai bnussman-akamai self-assigned this Oct 24, 2024
@bnussman-akamai bnussman-akamai requested a review from a team as a code owner October 24, 2024 16:16
@bnussman-akamai bnussman-akamai requested review from hana-akamai and coliu-akamai and removed request for a team October 24, 2024 16:16
@@ -57,7 +57,6 @@
"lib"
],
"devDependencies": {
"@types/node": "^12.7.1",
Copy link
Member Author

@bnussman-akamai bnussman-akamai Oct 24, 2024

Choose a reason for hiding this comment

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

I believe api-v4, validation, and ui don't need @types/node. These packages are intended to run in any environment / JS runtime and the source code don't use any Node.js specific features so I think we can remove the dependency.

The manager repo needs @types/node because we have scripts that use things like Node's "fs"

@bnussman-akamai bnussman-akamai requested a review from a team as a code owner October 24, 2024 16:31
@bnussman-akamai bnussman-akamai requested review from AzureLatte and removed request for a team October 24, 2024 16:31
Copy link

github-actions bot commented Oct 24, 2024

Coverage Report:
Base Coverage: 86.99%
Current Coverage: 86.99%

Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

Thanks Banks!

✅ not seeing any changes with the SessionExpirationDialog that uses useInterval
✅ general spot check of CM

@coliu-akamai coliu-akamai added the Approved Multiple approvals and ready to merge! label Oct 28, 2024
@bnussman-akamai bnussman-akamai merged commit 77111ab into linode:develop Oct 28, 2024
23 checks passed
Copy link

cypress bot commented Oct 28, 2024

Cloud Manager E2E    Run #6744

Run Properties:  status check failed Failed #6744  •  git commit 77111ab976: chore: [M3-8786] - Update and clean up `@types/node` package (#11157)
Project Cloud Manager E2E
Branch Review develop
Run status status check failed Failed #6744
Run duration 32m 16s
Commit git commit 77111ab976: chore: [M3-8786] - Update and clean up `@types/node` package (#11157)
Committer Banks Nussman
View all properties for this run ↗︎

Test results
Tests that failed  Failures 4
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 441
View all changes introduced in this branch ↗︎

Tests for review

Failed  kubernetes/lke-create.spec.ts • 3 failed tests

View Output Video

Test Artifacts
LKE Cluster Creation with ACL > with LKE IPACL account capability > creates an LKE cluster with ACL disabled Screenshots Video
LKE Cluster Creation with ACL > with LKE IPACL account capability > creates an LKE cluster with ACL enabled Screenshots Video
LKE Cluster Creation with ACL > with LKE IPACL account capability > can handle validation and API errors Screenshots Video
Failed  linodes/clone-linode.spec.ts • 1 failed test

View Output Video

Test Artifacts
clone linode > can clone a Linode from Linode details page Screenshots Video

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants