Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Eric_A created an issue. See original summary.

Eric_A’s picture

Title: Six quick composer fixes for our components » [quick fix] Trivial composer fixes for our components
Issue summary: View changes
Issue tags: +Quick fix
dsnopek’s picture

Is anyone hosting a subtree split somewhere that includes these changes?

dawehner’s picture

The DA has claimed the namespace, in order to actually host those, see https://packagist.org/packages/drupal/core-annotation and so on and forth.

Eric_A’s picture

Also, @Mile23 started this:

I made a travis-ci test of the components, which currently fails because of this issue. https://github.com/paul-m/drupal_component_tester

Travis results here: https://travis-ci.org/paul-m/drupal_component_tester

From #2867960-62: Merge Component composer.json files to account for them during build

dsnopek’s picture

What I meant is if someone had made their own subtree split with this patch so that you could put the alternate repos in "repositories" in your composer.json. Personally, I ended up making my own fork of drupal/core-annotations so that I could successfully install it in a non-Drupal project:

https://github.com/dsnopek/core-annotation/tree/issue-2876725

Anyway, I have to believe I'm not the only person trying to use that component outside of Drupal, so I was wondering if someone had already done it. :-)

joshmiller’s picture

Status: Needs review » Needs work
+++ b/core/tests/Drupal/Tests/ComposerIntegrationTest.php
@@ -47,6 +47,7 @@ protected function getPaths() {
       $this->root . '/core/lib/Drupal/Component/Assertion',
       $this->root . '/core/lib/Drupal/Component/Bridge',
+      $this->root . '/core/lib/Drupal/Component/ClassFinder',
       $this->root . '/core/lib/Drupal/Component/Datetime',
       $this->root . '/core/lib/Drupal/Component/DependencyInjection',

Why are we adding a dependency? Does this just need a re-roll without it?

pk188’s picture

Re rolled as patch failed to apply. Not sure about #8.

Status: Needs review » Needs work

The last submitted patch, 9: 2867960-24-drupal-8.3_0-9.patch, failed testing. View results

pk188’s picture

Status: Needs review » Needs work

The last submitted patch, 11: 2867960-24-drupal-8.3_0-11.patch, failed testing. View results

Eric_A’s picture

@pk188, let's only test against the branch each patch is targeted at :-)

+++ b/core/tests/Drupal/Tests/ComposerIntegrationTest.php
+++ b/core/tests/Drupal/Tests/ComposerIntegrationTest.php
@@ -47,6 +47,7 @@ protected function getPaths() {
       $this->root . '/core/lib/Drupal/Component/Assertion',
       $this->root . '/core/lib/Drupal/Component/Bridge',
+      $this->root . '/core/lib/Drupal/Component/ClassFinder',
       $this->root . '/core/lib/Drupal/Component/Datetime',
       $this->root . '/core/lib/Drupal/Component/DependencyInjection',

Why are we adding a dependency? Does this just need a re-roll without it?

@joshmiller, this hunk just fixes missing test coverage of one composer.json file. Whenever a component is added, we should expand this test, but obviously this is easily missed. We don't have a test to detect that we're not testing all composer.json files.

Eric_A’s picture

Note that differences between the branches is because #2769841: Prefer caret over tilde in composer.json went into 8.4.x.
Hunks that fix package names should not change the operator, i.e. the 8.4.x patch should now not accidentally change caret operators to tilde.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Eric_A’s picture

Title: [quick fix] Trivial composer fixes for our components » Fix trivial component composer bugs before 8.4.0
Priority: Normal » Major
Status: Needs work » Needs review
Issue tags: -Quick fix
FileSize
2.63 KB

It is actually becoming pretty embarrassing, such naming bugs, and then the obvious version conflict with core/composer.lock.

Eric_A’s picture

Title: Fix trivial component composer bugs before 8.4.0 » Fix trivial core component composer bugs before 8.4.0
alexpott’s picture

Status: Needs review » Needs work
+++ b/core/lib/Drupal/Component/Annotation/composer.json
@@ -6,9 +6,9 @@
     "doctrine/annotations": "1.2.*",

This should be ^1.2.*

Perhaps we need a test to ensure that these match what is in core/composer.json?

Mixologic’s picture

This patch is still fixing too many things that could have tests. If we pare it down to just the misspellings/wrong names, we can unblock drupalci's use of these components.

Eric_A’s picture

Status: Needs review » Reviewed & tested by the community

Yes, let's do just the name fixing, here.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I discussed this with @catch and we agreed that this could go into 8.4.x as well as 8.5.x

Committed and pushed 1188f1c to 8.5.x and b70d217 to 8.4.x. Thanks!

We need to open follow-ups for the other issues around consistent version requirements and test coverage.

  • alexpott committed 1188f1c on 8.5.x
    Issue #2876725 by Eric_A, pk188, Mixologic: Fix trivial core component...

  • alexpott committed b70d217 on 8.4.x
    Issue #2876725 by Eric_A, pk188, Mixologic: Fix trivial core component...
dsnopek’s picture

Huzzah! Thanks, Everyone!

Eric_A’s picture

We need to open follow-ups for the other issues around consistent version requirements (...)

Here's the issue for version conflicts:

#2913570: Version conflicts between core and embedded components requirements

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.