Problem/Motivation

In #2642636-5: Remove static Output in favor of injected OutputInterface we learn that some of our declared composer.json dependencies aren't actually dependencies.

The specific dependency (nickschuch/privatetravis) locks us to an earlier version of symfony/console.

What other horrors remain from MVP sprint time? :-)

Proposed resolution

Alter composer.json to remove unneeded dependencies.

Use the existing tests (after applying #2544340: Make the tests pass) to determine whether the dependency is needed.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#6 composer_cleanup_lock.patch51.13 KBjthorson
#6 composer-cleanup.patch8.69 KBjthorson
#2 2656446_2.patch672 bytesmile23

Comments

Mile23 created an issue. See original summary.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 2656446_2.patch672 bytes

Removed dependency on nickschuch/privatetravis.

Bumped symfony components up to 2.8.*

Removed dependency on composer/composer because I'm not sure why it's there.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

stage1/docker-php has a release: #2656480: Upgrade docker-php to latest version and refactor to use new managers

  • Mile23 committed de0204b on 2656446-evaluate-composer-json-needs 
    Issue #2656446 by Mile23: Evaluate composer.json needs
jthorson’s picture

jthorson commented
StatusFileSize
new composer-cleanup.patch8.69 KB
new composer_cleanup_lock.patch51.13 KB

We can cut deeper.

  • jthorson committed 4243d1a on 2656446-composer-cleanup 
    Issue #2656446 by jthorson, Mile23: Evaluate composer.json needs
jthorson’s picture

jthorson commented

Pushed up on 2656446-composer-cleanup branch.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Deleted branch 2656446-evaluate-composer-json-needs because it didn't include jthorson's work.

  • jthorson committed c627b5e on 2656446-composer-cleanup 
    Issue #2656446 by jthorson: Update composer.lock after merge
Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Status: Needs review » Needs work

The latest commit on this branch adds in merge conflicts.

jthorson’s picture

jthorson commented

Was it the composer.lock conflicts? Run composer update.

Updated branch pushed.

jthorson’s picture

jthorson commented
Status: Needs work » Needs review
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work 
$ git diff dev --name-only
composer.json
composer.lock
src/DrupalCI/Plugin/BuildSteps/configure/PrepResults.php
src/DrupalCI/Plugin/BuildSteps/publish/DrupalCIResults.php
src/DrupalCI/Plugin/JobTypes/simpletest/drupalci.yml

Why are we changing code? Seems out of scope here.

jthorson’s picture

jthorson commented
Status: Needs work » Reviewed & tested by the community

Oh ... I'm sorry. You're right ... the code changes ARE out of scope.

But if you look ... it's the complete removal of two obsolete classes and the already commented-out drupalci.yml lines which reference them. There's no actual code changes.

If we look at the actual diff, it feels kind of redundant to add them back in for this commit so that I can open a new issue just to remove them again.

jthorson’s picture

jthorson commented

Apparently PHP_Interpreter also snuck in there ... which was supposedly added back in Barcelona, and then somehow got committed back out.

EDIT: Here's the PHP_Interpreter RTBC link: #2568299-6: Switching between minor versions of PHP

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented

PHP_Interpreter got added to dev, somehow, so it got pulled down to everything else when dev got pulled down to those. It's an example of why we want to 'build the functional tests to catch the inevitable mistakes that everybody will make'.

jthorson’s picture

jthorson commented

In the interest of not having to re-run composer update every time I regress back to -dev ... can we get a 'go' here? Pretty please?

jthorson’s picture

jthorson commented

Removed guzzle/guzzle 3, updated code for guzzlehttp/guzzle 4, made dependency explicit in composer.json.

Branch is now passing all tests.

Edit: And to address #14, those classes were pulled out while removing the related drupal_api from the dependencies list.

jthorson’s picture

jthorson commented

Also updated unused 'download()' method in Patch.php to the guzzlehttp method signatures.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Status: Reviewed & tested by the community » Closed (fixed)

All sorts of convo happened in IRC. LGTM, merged into dev.