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: remove lockfiles #79

Merged
merged 1 commit into from May 9, 2022

Conversation

MichaelDeBoey
Copy link
Member

Follow-up of remix-run/remix#3110

Comment on lines +27 to +28
with:
useLockFile: false
Copy link
Member Author

Choose a reason for hiding this comment

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

@kentcdodds If you want, I can remove these again in remix.init

But I think it would be best to keep this as bahmutov/npm-install doesn't seem to support pnpm
https://github.com/bahmutov/npm-install#use-lock-file

Once bahmutov/npm-install#133 is implemented, I think we could reconsider this.

@kentcdodds kentcdodds merged commit 1d55df8 into remix-run:main May 9, 2022
@kentcdodds
Copy link
Member

Any idea for what we should do about this? https://github.com/remix-run/indie-stack/runs/6358741070?check_suite_focus=true

@MichaelDeBoey MichaelDeBoey deleted the remove-lockfiles branch May 9, 2022 20:12
@MichaelDeBoey
Copy link
Member Author

@kentcdodds Probably a good idea to remove package-lock.json from the Dockerfile?
If that's not preferred, we should install first I think 🤔

@kentcdodds
Copy link
Member

I don't think we want to remove it from the Dockerfile because users will want it in there.

@MichaelDeBoey
Copy link
Member Author

MichaelDeBoey commented May 9, 2022

@kentcdodds Then we should do an install first.

We can't know what file people will have though, as we don't know which package manager they will use.

remix-run/remix#3076 will already pass the used one to remix.init though, so if that one's merged/released, we could do ${packageManager} install & update Dockerfile in remix.init if we want to

@kentcdodds
Copy link
Member

Yeah, let's be a little more sophisticated 👍 I just merged that PR. Thanks!

@MichaelDeBoey
Copy link
Member Author

We'll need to wait for the next release to make it happen though. 😕

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