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

validate: suggest composer update --lock #8508

Closed
wants to merge 1 commit into from

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Jan 3, 2020

Suggest more safer composer update --lock command when lock file is not up to date.

Suggest more safer composer update --lock command when lock file is not up to date.
@glensc
Copy link
Contributor Author

glensc commented Jan 3, 2020

...or suggest both commands?

@svenluijten
Copy link
Contributor

svenluijten commented Jan 4, 2020

Similar to #8332. I guess if it wasn't updated there, we'd want to keep it the same here as well?

@glensc
Copy link
Contributor Author

glensc commented Jan 6, 2020

I find that people who don't know how composer work, want to bump only one dependency, edit composer.json, then found that project uses lock file, notice composer.lock is not updated, end up bumping all dependencies because composer tells them to run composer update, and then they don't really look composer.lock diff, commit whole lock file, and in the review process, it's typically ignored, because GitHub/GitLab by default hides such large diffs and reivewer sees only bumped dep on composer.json.

@Seldaek
Copy link
Member

Seldaek commented Jan 13, 2020

If it's an education problem then I hope my commit helps educating people in the right direction. update --lock is IMO not the solution to this.

@Seldaek Seldaek added this to the 1.9 milestone Jan 13, 2020
@Seldaek Seldaek closed this Jan 13, 2020
@glensc
Copy link
Contributor Author

glensc commented Jan 22, 2020

Indeed, if the lock file is out of date because someone used to edit package.json directly to bump a dependency, proper would for them to run composer update <package>.

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

3 participants