Skip to content
This repository has been archived by the owner on Aug 1, 2022. It is now read-only.

Use composer install to install dependencies #35

Merged
merged 2 commits into from
Apr 13, 2022

Conversation

dhensby
Copy link
Contributor

@dhensby dhensby commented Apr 13, 2022

This change uses composer install instead of composer update to install dependencies.

The point of the composer.lock file is to allow reproducible dependency installs across environments/projects. To use the update command during CI means that we get non-reproducible results and that we ship an untested lock file with our projects.

Not only this, but the use of update instead of install is having quite profound impacts on CI build times. Looking at the mfa module as an example, installation times have gone from 25 seconds to 3.5 minutes when switching from install to update.

Unless there's some undocumented reason as to why update is preferred to install, this should be changed

Copy link

@michalkleiner michalkleiner left a comment

Choose a reason for hiding this comment

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

There's no lock file to go by but using install over update still makes sense.

@emteknetnz
Copy link
Member

Makes sense. From memory there was a conscious reason why composer update made more sense than composer install, though I can't remember what that reason was. Possibly that reason is no longer be valid anyway.

@emteknetnz emteknetnz merged commit 974cbbe into silverstripe:master Apr 13, 2022
@dhensby dhensby deleted the pulls/composer-install branch April 13, 2022 23:29
@emteknetnz
Copy link
Member

OK probably the reason was that --prefer-lowest only works with composer update :D No worries, we can be a little dynamic with it - #36

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants