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: consider having a lockfile #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kiprasmel
Copy link

@kiprasmel kiprasmel commented Oct 4, 2021

NB! PR based on #42 which would need to get merged first.

don't know if you use npm or yarn - I chose yarn here but you can change it.

regardless, the lockfile should be committed, unless you have a good reason not to?

Signed-off-by: Kipras Melnikovas kipras@kipras.org

should've always been here, esp the build script.
had to read through [1] to figure it out smh

[1] https://medium.com/jspoint/a-simple-guide-to-load-c-c-code-into-node-js-javascript-applications-3fcccf54fd32

Signed-off-by: Kipras Melnikovas <kipras@kipras.org>
NB! PR based on LinusU#42 which would need to get merged first.

don't know if you use npm or yarn - I chose yarn here but you can change it.

regardless, the lockfile should be committed, unless you have a good reason not to?

Signed-off-by: Kipras Melnikovas <kipras@kipras.org>
@LinusU
Copy link
Owner

LinusU commented Oct 17, 2021

regardless, the lockfile should be committed, unless you have a good reason not to?

I think that best practice is to not commit the lock file for libraries that are published to npm. When a package is published to Npm the lock file is ignored, and thus you'll end up testing against older versions whereas people using your package will always use the latest...

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.

None yet

2 participants