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

Clean up dependencies #1248

Merged
merged 3 commits into from
Apr 12, 2024
Merged

Conversation

cheehongw
Copy link
Contributor

@cheehongw cheehongw commented Feb 19, 2024

Summary:

Fixes #1244
Fixes #1246
Fixes #1253

Changes Made:

See discussion in summary for more details

  • Remove wait-on as devDep
  • Added importHelpers: true in tsconfig.json as part of tslib
  • Remove node-fetch as dependency

Proposed Commit Message:

Instead of a squash & merge, should we do a normal merge and keep all 3 atomic commits made in this PR?

Clean up dependencies

- Remove `wait-on` as devDep
- Added `importHelpers: true` in `tsconfig.json` as part of `tslib`
- Remove `node-fetch` as dependency

@cheehongw cheehongw marked this pull request as draft February 19, 2024 13:50
@cheehongw cheehongw force-pushed the clean-up-deps branch 2 times, most recently from b347eac to 65f638e Compare February 19, 2024 17:20
@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.88%. Comparing base (944a424) to head (45aa199).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1248      +/-   ##
==========================================
+ Coverage   54.82%   58.88%   +4.05%     
==========================================
  Files         105      105              
  Lines        2858     2573     -285     
  Branches      503      288     -215     
==========================================
- Hits         1567     1515      -52     
+ Misses       1030     1008      -22     
+ Partials      261       50     -211     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cheehongw cheehongw marked this pull request as ready for review February 19, 2024 17:37
@cheehongw cheehongw marked this pull request as draft February 20, 2024 02:09
@cheehongw cheehongw marked this pull request as ready for review March 4, 2024 04:49
@cheehongw cheehongw mentioned this pull request Mar 4, 2024
Copy link
Contributor

@luminousleek luminousleek left a comment

Choose a reason for hiding this comment

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

LGTM! And yes I think we can have it as atomic commits instead of squashing and merging to make rolling back easier if we need to.

wait-on doesn't seem to be used at all in our workflows. Let's remove
it.
In order to use tslib, we need to include this compiler option in
tsconfig. Based off clean installs of angular 12 projects via ng new,
this setting appears to be the default as well.

See:
- https://angular.io/guide/typescript-configuration#tsconfig
- From https://www.npmjs.com/package/tslib
CATcher does not directly depend on node-fetch. After the removal of
electron, there are no more files depending on node-fetch.
@luminousleek luminousleek merged commit 05fe528 into CATcher-org:master Apr 12, 2024
5 checks passed
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.

Remove node-fetch as direct dependency tslib unused Remove wait-on from devDep
3 participants