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

Node package manager improvements #9426

Merged
merged 5 commits into from
Jan 16, 2025
Merged

Node package manager improvements #9426

merged 5 commits into from
Jan 16, 2025

Conversation

sschuberth
Copy link
Member

This centralizes project creation which will ease an upcoming change.

Make more clear that these files host functions that are for any Node
package manager, not just NPM specifically.

Also rename the `NodePackageManager` enum to `NodePackageManagerType` to
make room for another class that is more appropriate to use that name.

Signed-off-by: Sebastian Schuberth <[email protected]>
Prepare for generalizing some code.

Signed-off-by: Sebastian Schuberth <[email protected]>
Put code that is common to all Node package managers into an abstract
base class to reduce code duplication and to ease upcoming changes. As
some types are now exposed through the class hierarchy / constructor
parameters, it is required to make them public. While at it, also
perform some minor code alignments.

Note that this reintroduction of a class hierarchy does not go against
the idea of recently made changes to decouple the individual Node
package manager implementations from each other: It is still the case
that e.g. the PNPM implementation should not inherit from the NPM
implementation.

Signed-off-by: Sebastian Schuberth <[email protected]>
This avoids the need to pass the `managerName` as a parameter, and to
have a top-level logger.

Signed-off-by: Sebastian Schuberth <[email protected]>
@sschuberth sschuberth changed the title refactor(analyzer): Add a function to create dependency graph projects Node package manager improvements Jan 15, 2025
@sschuberth sschuberth marked this pull request as ready for review January 15, 2025 21:37
@sschuberth sschuberth requested a review from a team as a code owner January 15, 2025 21:37
@sschuberth sschuberth enabled auto-merge (rebase) January 15, 2025 21:37
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

Attention: Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.

Project coverage is 68.07%. Comparing base (85defa3) to head (ab452ad).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
...s/node/src/main/kotlin/npm/NpmDependencyHandler.kt 50.00% 1 Missing ⚠️
...e-managers/node/src/main/kotlin/pnpm/ModuleInfo.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9426      +/-   ##
============================================
- Coverage     68.07%   68.07%   -0.01%     
+ Complexity     1291     1285       -6     
============================================
  Files           249      249              
  Lines          8837     8827      -10     
  Branches        918      918              
============================================
- Hits           6016     6009       -7     
+ Misses         2435     2432       -3     
  Partials        386      386              
Flag Coverage Δ
funTest-docker 65.00% <90.32%> (-0.03%) ⬇️
funTest-non-docker 33.36% <ø> (ø)
test-ubuntu-24.04 35.89% <45.16%> (+0.02%) ⬆️
test-windows-2022 35.87% <45.16%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@sschuberth
Copy link
Member Author

@fviernau agreed privately to proceed with this without his review.

@sschuberth sschuberth merged commit 66b5b15 into main Jan 16, 2025
26 checks passed
@sschuberth sschuberth deleted the create-proj-helper branch January 16, 2025 10:24
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.

2 participants