- Status changed to Needs review
almost 2 years ago 11:54am 17 January 2023 - Status changed to Needs work
almost 2 years ago 3:50pm 17 January 2023 - Assigned to tedbow
- Status changed to Needs review
almost 2 years ago 6:37pm 19 January 2023 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Merged in
8.x-2.x
, which includes π Improve test DX *and* confidence: stop using VFS Fixed . The changes that were made there made it all the more awkward to make the changes here that π Reliably support cweagans/composer-patches in Package Manager & Automatic Updates: validate stage Fixed should make. So β¦ removed them, and instead commented out the ~20 LoC that will ensure full test coverage of this issue's new validator combined withComposerPatchesValidator
.And that made me realize that in fact this issue should move forward already without π Reliably support cweagans/composer-patches in Package Manager & Automatic Updates: validate stage Fixed having landed, because this makes it more clear in what way HEAD's
ComposerPatchesValidator
is not yet dealing with all edge cases. So: pushed3bdc3ea
to uncouple it and now unpostponing! - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Core policy follow-up created: π [Policy, no patch] Projects depending on composer plugins will have to update the additional_trusted_composer_plugins setting in package_manager.settings Active .
- Status changed to RTBC
almost 2 years ago 6:45pm 23 January 2023 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
This is ready for final review from @tedbow.
- Assigned to wim leers
- Status changed to Needs work
almost 2 years ago 6:24pm 24 January 2023 - πΊπΈUnited States tedbow Ithaca, NY, USA
Needs work for MR question
- Status changed to RTBC
almost 2 years ago 1:45pm 26 January 2023 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Responded on MR. Thank you. This made me realize we need a new issue, which actually is technically unrelated to this issue, but it was definitely discovered thanks to this issue β tagging to ensure we don't forget π
- Issue was unassigned.
- Status changed to Postponed
almost 2 years ago 5:05pm 26 January 2023 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Let's wait a moment here, because I think a lot of the stuff this had to do could land in π Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers) Fixed β¦
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Created one more
core-post-mvp
follow-up: π Identify & vet commonly used composer plugins in the Drupal ecosystem Active . - Assigned to wim leers
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Discussed with @tedbow in detail. π Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers) Fixed will incorporate the changes to
Build
tests that this issue made. That means this issue is now officially postponed on that one.All follow-ups were created.
Once π Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers) Fixed lands, the changes in this MR will be significantly smaller and easier to review, so let's wait for that π Assigning to me to update it after that lands. π
- Status changed to Needs work
almost 2 years ago 1:20am 31 January 2023 - πΊπΈUnited States tedbow Ithaca, NY, USA
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Rebased on top of π Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers) Fixed .
- Issue was unassigned.
- Status changed to RTBC
almost 2 years ago 12:25pm 31 January 2023 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
And now removed the remaining out-of-scope-but-discovered-here changes from this MR and adding them to π Assert no errors after creating the test project in ModuleUpdateTest Needs review π
- πΊπΈUnited States tedbow Ithaca, NY, USA
create follow-up π Do not allow drupal/core-composer-scaffold to be used by packages other than core Fixed
- Assigned to wim leers
- Status changed to Needs work
almost 2 years ago 5:22pm 1 February 2023 - πΊπΈUnited States tedbow Ithaca, NY, USA
Needs work for MR comments
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Discusssed with @tedbow, I'm 70% of the way through implementing this β I thought it'd be trivial but because now we need to modify
composer.json
, I also need to modify theFixtureManipulator
β¦ π¬Probably will be unable to finish this tonight; almost need to run for dinner. Definitely to be continued tomorrow!
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Got it working, but still need to expand the test coverage for the new possible failure modes tomorrow.
- Assigned to tedbow
- Status changed to RTBC
almost 2 years ago 1:18pm 3 February 2023 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Addressed all of @tedbow's feedback, test coverage expanded accordingly, and found a critical bug in my test coverage while doing so π
Also thoroughly documented the rationale behind the architecture in the class-level docblock, so that the results of the conversatino @tedbow and I had are available for posterity π
- Assigned to wim leers
- Status changed to Needs work
almost 2 years ago 3:20am 6 February 2023 - πΊπΈUnited States tedbow Ithaca, NY, USA
Needs work at least to use the Composer Inspector
- Status changed to RTBC
almost 2 years ago 7:53pm 6 February 2023 - πΊπΈUnited States tedbow Ithaca, NY, USA
RTBC
@Wim Leers if I didn't address your question well enough in https://git.drupalcode.org/project/automatic_updates/-/merge_requests/65...
lets make a follow-up
-
tedbow β
committed 9bfc8ace on 8.x-2.x authored by
Wim Leers β
Issue #3331168 by Wim Leers, tedbow: Limit trusted Composer plugins to a...
-
tedbow β
committed 9bfc8ace on 8.x-2.x authored by
Wim Leers β
- Status changed to Fixed
almost 2 years ago 7:56pm 6 February 2023 - Issue was unassigned.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
@tedbow Iβm really glad that this landed, but I worry that introducing version restrictions on the supported composer plugins later will constitute a BC break β not just for the configuration, but also for the behavior of the "out of the box supported" composer plugins.
Is that not a concern for you?
Either way, it's good to have this landed, we can add version constraints in a next issue. And I agree with your remark that you posted just prior to committing:
\Drupal\package_manager\Validator\ComposerPatchesValidator
should be markedfinal
and@internal
. Perhaps we can do all of that combined in acore-mvp
follow-up? - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Discussed #43: π Tighten ComposerPluginsValidator: support only specified version constraint Fixed .
Automatically closed - issue fixed for 2 weeks with no activity.