CLI-1181: [pull:code] disable scripts for build branches #1614
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1614 +/- ##
============================================
+ Coverage 91.81% 91.83% +0.02%
- Complexity 1815 1816 +1
============================================
Files 124 124
Lines 6498 6505 +7
============================================
+ Hits 5966 5974 +8
+ Misses 532 531 -1
☔ View full report in Codecov by Sentry. |
| $this->checklist->completePreviousItem(); | ||
| if (!file_exists(Path::join($this->dir, 'composer.json'))) { | ||
| $this->logger->notice('composer.json file not found. Skipping composer install.'); | ||
| return; |
There was a problem hiding this comment.
Negative exists logic here seems to make sense, I think.
There was a problem hiding this comment.
I like to do any validation at the start of the method, hence the negative logic. I'm open to alternatives.
| } | ||
| $this->checklist->addItem("Installing Composer dependencies"); | ||
| $this->composerInstall($outputCallback); | ||
| $this->checklist->completePreviousItem(); |
There was a problem hiding this comment.
Not sure if the mutation tests are catching something here or if they just need to be updated. It seems it's the latter.
There was a problem hiding this comment.
Due to idiosyncrasies with how the Symfony Command Tester class handles output, our test cases can't verify that these checklist items appear in the command output. Hence the escaped mutant. But it's not caused by this PR, so we can ignore it.
|
This is ready for review (we even increased test coverage and MSI! 😮 ) |
Motivation
For certain applications and environments,
composer installcan fail when composer dependencies are already installedProposed changes
Don't run composer install if dependencies are already installed; i.e., for build branches.