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

DT-1399: Evaluate Composer 2.0 readiness. #4088

Merged
merged 9 commits into from
May 7, 2020
Merged

Conversation

@danepowell danepowell marked this pull request as ready for review May 7, 2020 15:34
@danepowell danepowell merged commit 56a5cda into acquia:12.x May 7, 2020
"composer/semver": "^1.4",
"consolidation/comments": "^1.0",
"consolidation/config": "^1.0.0",
"consolidation/robo": "^1.4.12 || ^2",
"cweagans/composer-patches": "^1.6.5",
Copy link
Member

@wimleers wimleers May 11, 2020

Choose a reason for hiding this comment

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

This is a huge BC break and caused more than a dozen of hours of time spent debugging acquia/acquia_migrate#184 😭

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry to cause you grief, but just to be clear this only happened in the 12.x branch which is explicitly unstable and unsupported. However we just cut 12.0.0-alpha1 and I don't foresee any more BC breaks, although they could still happen.

Copy link
Contributor Author

@danepowell danepowell May 11, 2020

Choose a reason for hiding this comment

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

Also I suppose this is an example of the paradigm that if you rely on a dependency, you should declare it, even if you already get it "for free" from some upstream package. You might want to see what else BLT is providing that you rely on, and make sure that you explicitly declare the dependency yourself.

Copy link
Member

Choose a reason for hiding this comment

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

but just to be clear this only happened in the 12.x branch which is explicitly unstable and unsupported

But we're explicitly building a stable product on top of Drupal 9 over at https://github.com/acquia/migrate.

We must use https://github.com/acquia/orca.

https://github.com/acquia/orca requires us to use https://github.com/acquia/blt, and specifically version 12 to test with Drupal 9.

So I don't see how we have any choice here?

I think we can be pragmatic here: explicitly ping me whenever there's a BC break being introduced in BLT 12, so that we can make whatever changes we need to not break the build, or only break it for a brief period of time :)

Copy link
Member

Choose a reason for hiding this comment

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

Also I suppose this is an example of the paradigm that if you rely on a dependency, you should declare it, even if you already get it "for free" from some upstream package.

This is totally fair! :)

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.

2 participants