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

Feature/grumphp upgrade #54

Merged
merged 13 commits into from
Dec 8, 2020
Merged

Conversation

mitrpaka
Copy link
Contributor

This PR fixes issue #52.

Upgrades tasks as documented in Grumphp release notes (0.18 || 0.19)

Extended external task class

    Make getConfigurableOptions() static
    Change service configuration + tag (config -> task)

Extended an external task from TaskInterface

    Make getConfigurableOptions() static
    Register an EmptyTaskConfig during construction
    Replace getConfiguration() method by the getConfig() method
    Add immutable withConfig() method.
    Change service configuration + tag (config -> task)

Duplicate task with different config

    Remove task copy
    Remove service configuration of task copy
    Configure task copy as an alias : https://github.com/phpro/grumphp/blob/master/doc/tasks.md#run-the-same-task-twice-with-different-configuration

@tormi tormi self-requested a review December 3, 2020 07:57
Copy link
Member

@tormi tormi left a comment

Choose a reason for hiding this comment

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

RTBC, tested here: wunderio/drupal-project#165 Thanks, @mitrpaka !

@tormi
Copy link
Member

tormi commented Dec 3, 2020

Looks like there's a packages mismatch when trying to install this with drupal/core-recommended 8.9.10:

lando composer require wunderio/code-quality:dev-feature/grumphp-upgrade --dev 
./composer.json has been updated
Running composer update wunderio/code-quality
Gathering patches for root package.
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - phpro/grumphp[v1.0.0, ..., v1.2.0] require doctrine/collections ^1.6.7 -> found doctrine/collections[1.6.7, 1.6.x-dev, 1.7.x-dev] but the package is fixed to v1.4.0 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
    - wunderio/code-quality dev-feature/grumphp-upgrade requires phpro/grumphp ^1 -> satisfiable by phpro/grumphp[v1.0.0, v1.1.0, v1.2.0].
    - Root composer.json requires wunderio/code-quality dev-feature/grumphp-upgrade -> satisfiable by wunderio/code-quality[dev-feature/grumphp-upgrade].

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.

Installation failed, reverting ./composer.json and ./composer.lock to their original content.
lando composer why doctrine/collections
commerceguys/addressing  v1.1.0  requires  doctrine/collections (~1.0)    
doctrine/common          v2.7.3  requires  doctrine/collections (1.*)     
drupal/core-recommended  8.9.10  requires  doctrine/collections (v1.4.0)  

@tormi tormi self-requested a review December 4, 2020 09:26
Copy link
Member

@tormi tormi left a comment

Choose a reason for hiding this comment

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

This breaks current Drupal 8.9 sites:

Problem 1
    - phpro/grumphp[v1.0.0, ..., v1.2.0] require doctrine/collections ^1.6.7 -> found doctrine/collections[1.6.7, 1.6.x-dev, 1.7.x-dev] but the package is fixed to v1.4.0 (lock file version)

drupal/core-recommended 8.9.10 requires doctrine/collections (v1.4.0), https://github.com/drupal/core-recommended/blob/8.9.x/composer.json#L15

#54

D 9 sites are fine because doctrine/collections requirement has been removed from drupal/core-recommended.

@tormi
Copy link
Member

tormi commented Dec 8, 2020

This breaks current Drupal 8.9 sites

We've discussed this with @guncha25 last week and agreed that the solution would be to introduce new MAJOR version for D9 with this PR. All D 8 sites will miss the Composer 2 goodies, but let's consider this as a motivation to upgrade to D9.

@tormi
Copy link
Member

tormi commented Dec 8, 2020

@guncha25 also mentioned that this still needs some work from him before it can be merged.

@guncha25 guncha25 merged commit 38c5922 into wunderio:master Dec 8, 2020
@tormi
Copy link
Member

tormi commented Dec 8, 2020

👍

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.

3 participants