-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
improve node management #1153
improve node management #1153
Conversation
ryoppippi
commented
Dec 13, 2024
- manage pnpm version by pnpm
- manage node version by ci
- install node & pnpm using pnpm itself in ci env
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think nestia
needs matrix strategy of NodeJS versions. Others are good to see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, ignoring pnpm-lock.yaml
file is inteded.
As this is a library repository, I've always test the ^
version setup startegy.
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is a typosquat?Package name is similar to other popular packages and may not be the package you want. Use care when consuming similarly named packages and ensure that you did not intend to consume a different package. Malicious packages often publish using similar names as existing popular packages. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
I think including lockfile in Node project is fine, because the library version is updated by renovate every time.
We need lockfile because of development. |
I'll add matrix later |
So managing node version by pnpm is not good for this case? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As nestia
utilizes many NodeJS built-in functions, I think NodeJS version matrix is valid for this repo.
Also, do not change too much style please. Ignoring pnpm-lock.yaml
file is required for dependencies' patch and minor updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your endless contribution.
There may be a better and more superior solution, or I may have made a wrong choice.
However, the changes must be made at a level that I can keep going the development.
pnpm-lock.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must be removed with below commented reason.
.gitignore
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also change please
- name: Build | ||
working-directory: website | ||
run: npm install && npm run build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, website must use npm
at now.
This may be caused by my unskilled monorepo composition, anyway, if try to use pnpm install
command in the website
directory, it does not install of website
, but of other modules enrolled in the pnpm-workspace.yaml
file.
By the way, if add the website
directory to the pnpm-workspace.yaml
file is impossible. It is because the website
module requires other module's complete compilation and testing.
I'll create another PR based on this! |