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

Minor update to Frontend Readme #172

Merged
merged 1 commit into from
Apr 21, 2023
Merged

Minor update to Frontend Readme #172

merged 1 commit into from
Apr 21, 2023

Conversation

Jooms
Copy link
Contributor

@Jooms Jooms commented Apr 20, 2023

Updating this to the new format that the backend/README.md uses

Resolves #51

@Jooms Jooms mentioned this pull request Apr 20, 2023
@hardikpatil hardikpatil self-requested a review April 20, 2023 15:39
Copy link
Contributor

@hardikpatil hardikpatil left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@decoles decoles self-requested a review April 20, 2023 16:30
@decoles
Copy link
Contributor

decoles commented Apr 20, 2023

LGTM

@sathyanarayanan-v
Copy link
Contributor

Can we have npm ci instead of npm install to make sure we don't update packages accidentally?

@Jooms
Copy link
Contributor Author

Jooms commented Apr 21, 2023

npm ci

From: https://docs.npmjs.com/cli/v9/commands/npm-ci

Is only meant to be used in things like CI, testing, etc. Meant to be a clean install of dependencies. From my understanding npm install is the correct command to use for development.

I do understand what you're calling out though. npm install will pull the latest minor / patch versions for each dependency in the package.json file, which can (and likely will) cause the package-lock.json file to change every time. I assume there is an industry / common way of handling this in full-fledged JS teams, but it seems that npm-ci isn't it (at least with the documentation I see).

There's definitely room for discussion on "How do we improve the rampant merge conflicts we run into with package-lock.json". Especially when documentation shows that 1) we're supposed to keep package-lock.json committed, and 2) npm install is supposed to be used for development environments.

Package-lock.json docs: https://docs.npmjs.com/cli/v8/configuring-npm/package-lock-json

This file is intended to be committed into source repositories, and serves various purposes ...

Please create a Discussion on this topic if you'd like to discuss it more.

@Jooms Jooms merged commit f7f538f into main Apr 21, 2023
@Jooms Jooms deleted the krepelka/frontendReadme branch April 21, 2023 18:10
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.

Clean up README formatting
4 participants