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

upgrade crop module dependency. #181

Open
wants to merge 9 commits into
base: 8.x-1.x
Choose a base branch
from

Conversation

stopfstedt
Copy link

Hi,

We'd like to upgrade to the latest version of Crop in the 8.x-2.x branch, this will allow us to provide the Sitefarm profile alongside Acquia Lightning profile in our environment.

Currently, we're running into a dependency conflict with Crop module, so this should fix this.

Thanks!

Copy link
Contributor

@mrkmiller mrkmiller left a comment

Choose a reason for hiding this comment

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

You can't put docroot because it may not always be docroot. Some use web instead of docroot. As it is, I don't believe this code actually is parsed since it is a dependency. Your root distribution composer.json is where you need to set these.

@eguerin-ucsf
Copy link
Collaborator

Yeah unfortunately it seems to manifest as a problem with the merge-extras param in the composer-merge-plugin set to true. I want to have the extras merged between Lightning, Sitefarm_seed and the parent composer.json because they all have various patches that need to be applied. What I find out from this is then it takes the installer-paths from sitefarm_seed which messes up the installation.

Since we are currently applying sitefarm_seed using this stopfstedt version I can set the path, but yes I can see it being an issue. My recommendation would be to remove all installer-paths from the sitefarm_seed composer file. As all the paths should be predefined in the sitefarm-distro composer file anyhow?

Would anyone be willing to review the issues surrounding the Crop update, I don't have access to your ProboCI environment.

@cfblack
Copy link
Contributor

cfblack commented May 2, 2018

@eguerin-ucsf Probo.ci reports

Drupal\Core\Config\PreExistingConfigException: Configuration objects [error]
69(system.theme) provided by system already exist in active
70configuration in
71/src/sitefarm-distro-template/web/core/lib/Drupal/Core/Config/PreExistingConfigException.php:65
72Stack trace:

etc... during the Drupal site setup
This appears to be a reoccurring issue on pull requests. Probably something that needs to be handled with a change to .probo.yml, not necessarily a problem with your PR.

@mrkmiller
Copy link
Contributor

Please resolve the merge conflicts and limit the scope of this to simply updating the crop module.

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.

4 participants