Problem/Motivation

At some point we need to update to symfony 3.0, as this will be the only supported version.

Proposed resolution

  • For now let's move to 2.8 to cover all new features / deprecations.
  • Commit this patch to 8.1
  • Announce the update to 2.8
  • Announce the update to 3.0 in 8.2, so custom/contrib code can adapt to it

Other commends

Problems with update to 3.0 identified as part of this issue:

Let's experiment whether/how we can update.

Issues/problems identified:

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#88 update_to_symfony_2_8-2611816-88-quickfix.patch482 bytesneclimdul
#80 interdiff.txt1.72 KBdawehner
#80 2611816-80.patch777.04 KBdawehner
#78 interdiff-2611816-67-78.txt936 bytesAnkit Agrawal
#78 2611816-78.patch784.8 KBAnkit Agrawal
#74 interdiff.txt837 bytescatch
#74 2611816-74.patch776.6 KBcatch
#71 2611816-67.patch775.79 KBcatch
#67 2611816-review.patch61.59 KBdawehner
#67 interdiff.txt2.12 KBdawehner
#67 2611816-67.patch776.66 KBdawehner
#64 2611816-core-only-do-not-test.patch60.8 KBjibran
#63 2611816-63.patch776.53 KBdawehner
#60 2611816-60.patch24.22 KBjibran
#60 interdiff.txt483 bytesjibran
#59 2611816-59.patch24.22 KBjibran
#54 2611816-54.patch809.56 KBdawehner
#54 2611816-54.patch809.56 KBdawehner
#52 2611816-52.patch809.56 KBdawehner
#50 2611816-50.patch1.56 MBdawehner
#47 2611816-47.patch24.13 KBdawehner
#42 2611816-42.patch1.48 MBhussainweb
#42 interdiff-35-42.txt715 byteshussainweb
#35 core.txt24.12 KBdawehner
#35 2611816-35.patch1.48 MBdawehner
#31 core.patch29.38 KBdawehner
#30 interdiff.txt2.31 KBdawehner
#30 2611816-30.patch1.48 MBdawehner
#28 interdiff.txt837 bytesdawehner
#28 2611816-28.patch1.48 MBdawehner
#24 2611816-22.patch1.48 MBdawehner
#24 interdiff.txt14.74 KBdawehner
#17 interdiff.txt151.59 KBdawehner
#17 2611816-17.patch1.46 MBdawehner
#13 interdiff.txt95.86 KBdawehner
#13 2611816-13.patch1.44 MBdawehner
#11 interdiff.txt3.66 KBdawehner
#11 2611816-11.patch1.39 MBdawehner
#9 interdiff.txt56.06 KBdawehner
#9 2611816-9.patch1.39 MBdawehner
#5 2611816-5.patch2.26 MBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

First problem:

  Problem 1
    - stack/builder v1.0.3 requires symfony/http-foundation ~2.1 -> satisfiable by symfony/http-foundation[2.1.x-dev, 2.2.x-dev, 2.3.x-dev, 2.4.x-dev, 2.5.x-dev, 2.6.x-dev, 2.7.x-dev, 2.8.x-dev].
    - stack/builder v1.0.0 requires symfony/http-foundation ~2.1 -> satisfiable by symfony/http-foundation[2.1.x-dev, 2.2.x-dev, 2.3.x-dev, 2.4.x-dev, 2.5.x-dev, 2.6.x-dev, 2.7.x-dev, 2.8.x-dev].
    - stack/builder v1.0.1 requires symfony/http-foundation ~2.1 -> satisfiable by symfony/http-foundation[2.1.x-dev, 2.2.x-dev, 2.3.x-dev, 2.4.x-dev, 2.5.x-dev, 2.6.x-dev, 2.7.x-dev, 2.8.x-dev].
    - stack/builder v1.0.2 requires symfony/http-foundation ~2.1 -> satisfiable by symfony/http-foundation[2.1.x-dev, 2.2.x-dev, 2.3.x-dev, 2.4.x-dev, 2.5.x-dev, 2.6.x-dev, 2.7.x-dev, 2.8.x-dev].
    - stack/builder v1.0.3 requires symfony/http-foundation ~2.1 -> satisfiable by symfony/http-foundation[2.1.x-dev, 2.2.x-dev, 2.3.x-dev, 2.4.x-dev, 2.5.x-dev, 2.6.x-dev, 2.7.x-dev, 2.8.x-dev].
    - Can only install one of: symfony/http-foundation[dev-master, 2.5.x-dev].
    - Can only install one of: symfony/http-foundation[dev-master, 2.6.x-dev].
    - Can only install one of: symfony/http-foundation[dev-master, 2.7.x-dev].
    - Can only install one of: symfony/http-foundation[dev-master, 2.8.x-dev].
    - Can only install one of: symfony/http-foundation[dev-master, 2.1.x-dev].
    - Can only install one of: symfony/http-foundation[dev-master, 2.2.x-dev].
    - Can only install one of: symfony/http-foundation[dev-master, 2.3.x-dev].
    - Can only install one of: symfony/http-foundation[dev-master, 2.4.x-dev].
    - Installation request for symfony/http-foundation dev-master -> satisfiable by symfony/http-foundation[dev-master].
    - Installation request for stack/builder 1.0.* -> satisfiable by stack/builder[v1.0.0, v1.0.1, v1.0.2, v1.0.3].
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Issue summary: View changes

Next problem:

  Problem 1
    - symfony-cmf/routing 1.3.0 requires symfony/http-kernel ~2.2 -> satisfiable by symfony/http-kernel[2.2.x-dev, 2.3.x-dev, 2.4.x-dev, 2.5.x-dev, 2.6.x-dev, 2.7.x-dev, 2.8.x-dev].
    - symfony-cmf/routing 1.3.0 requires symfony/http-kernel ~2.2 -> satisfiable by symfony/http-kernel[2.2.x-dev, 2.3.x-dev, 2.4.x-dev, 2.5.x-dev, 2.6.x-dev, 2.7.x-dev, 2.8.x-dev].
    - Can only install one of: symfony/http-kernel[dev-master, 2.2.x-dev].
    - Can only install one of: symfony/http-kernel[dev-master, 2.3.x-dev].
    - Can only install one of: symfony/http-kernel[dev-master, 2.4.x-dev].
    - Can only install one of: symfony/http-kernel[dev-master, 2.5.x-dev].
    - Can only install one of: symfony/http-kernel[dev-master, 2.6.x-dev].
    - Can only install one of: symfony/http-kernel[dev-master, 2.7.x-dev].
    - Can only install one of: symfony/http-kernel[dev-master, 2.8.x-dev].
    - Installation request for symfony/http-kernel dev-master -> satisfiable by symfony/http-kernel[dev-master].
    - Installation request for symfony-cmf/routing 1.3.* -> satisfiable by symfony-cmf/routing[1.3.0].
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Active » Needs review
FileSize
2611816-5.patch2.26 MB

Let's ask the testbot.

Status: Needs review » Needs work

The last submitted patch, 5: 2611816-5.patch, failed testing.

cosmicdreams’s picture

cosmicdreams CreditAttribution: cosmicdreams as a volunteer commented

Hey @dawehner I'm hearing form PHP folks that https://github.com/symfony/symfony/issues/9406 might be a significant issue as it's the "biggest backwards compatibility break" from their opinion.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

It wouldn't matter much because we just use the http kernel side of http kernel, none of the other stuff.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Title: [experiment] Update to symfony 3.0 » [experiment] Update to symfony 2.8
Status: Needs work » Needs review
FileSize
2611816-9.patch1.39 MB
interdiff.txt56.06 KB

Uploading an update to symfony 2.8

Status: Needs review » Needs work

The last submitted patch, 9: 2611816-9.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Issue summary: View changes
FileSize
2611816-11.patch1.39 MB
interdiff.txt3.66 KB

Listing a couple of problems in the issue summary.

clifford8’s picture

clifford8 CreditAttribution: clifford8 commented
Assigned: Unassigned » clifford8
Issue tags: +need work
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Version: 8.0.x-dev » 8.1.x-dev
Assigned: clifford8 » Unassigned
Status: Needs work » Needs review
FileSize
2611816-13.patch1.44 MB
interdiff.txt95.86 KB

Let's update with some fixes.

Status: Needs review » Needs work

The last submitted patch, 13: 2611816-13.patch, failed testing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Title: [experiment] Update to symfony 2.8 » [experiment] Update to symfony 3.0
Priority: Normal » Major

It's out: http://symfony.com/blog/symfony-3-0-0-released

Would be great to get this into 8.1.x sooner than later.

Also would be completely fine with an incremental update to 2.8.0, then a further issue to get from there to 3.0.0

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

2.8 to 3.0 should be trivial, because 2.8 throws all the deprecation notices, but yeah I don't care which one to target.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs work » Needs review
FileSize
2611816-17.patch1.46 MB
interdiff.txt151.59 KB

Updated to the 2.8 release, this should fix most of the issues, but still not all of them.

Status: Needs review » Needs work

The last submitted patch, 17: 2611816-17.patch, failed testing.

The last submitted patch, 17: 2611816-17.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented 
Drupal\system\Tests\Database\QueryTest->Drupal\system\Tests\Database\{closure}(16384, 'The "entity.manager" service relies on the deprecated "Drupal\Core\Entity\EntityManager" class. It should either be deprecated or its implementation upgraded.', '/Users/dawehner/www/d8/vendor/symfony/dependency-injection/ContainerBuilder.php', 926, Array)
trigger_error('The "entity.manager" service relies on the deprecated "Drupal\Core\Entity\EntityManager" class. It should either be deprecated or its implementation upgraded.', 16384)
Symfony\Component\DependencyInjection\ContainerBuilder->createService(Object, 'entity.manager')

This is actually really crazy. We kinda have a problem I guess?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

So this is http://symfony.com/blog/new-in-symfony-2-8-deprecated-service-definitions

But then we'll get a different deprecation notice when that service is instantiated.

In Drupal we've got a distinction between marking something @deprecated, and using trigger_error() per #2575081: [policy, no patch] Use E_USER_DEPRECATED in Drupal 8 minor releases - so that we can give people the earliest possible warning of a deprecation, then start showing errors in a future minor release.

So we're going to need to disable Symfony's deprecation framework as far as I can see - at least for now.

Then if we want to use it later, we'd need to be able to add an opt-out to the trigger_error() in service definitions.

Something like

deprecated: ~
warn: nope
pounard’s picture

pounard
Primary language French
CreditAttribution: pounard commented

If the goal to pursue is to upgrade to SF3, all deprecation warnings must be fixed, not silenced, since the 2.8 deprecation warnings are explicitly telling you what have been removed in 3.0. I myself did that migration from 2.7 to 3.0 in personal SF projects, 2.8 and 3.0 are ISO in everything except that 3.0 removes all deprecated features, and 2.8 keep them only in the goal of making migration from 2.7 easier.

Drupal should not upgrade to 2.8 IMHO, I think this issue is doing it right by first upgrading to 2.8 in a temporary fashion, just the time to fix all the deprecation warnings, once done, Drupal will be able to seamlessly go to 3.0 in this very same issue without ever 2.8 being commited in any of the 8.x branch(es).

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

@pounard if you read the error, it's not a Symfony 2.8 deprecation but a Drupal deprecation we added just before release. We explicitly do not want to trigger errors for these yet to give contrib and custom code time to update. Similarly Symfony did not start triggering errors for deprecations until 2.8.

The problem is that deprecations we've added are being picked up by Symfony's deprecation system, not that we're using deprecated Symfony classes.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
FileSize
interdiff.txt14.74 KB
2611816-22.patch1.48 MB

First I think we should actually support deprecated services, which we didn't see the interdiff. Once we are there, we can explicitly opt out something.

pounard’s picture

pounard
Primary language French
CreditAttribution: pounard commented

@catch Oh I didn't catch that, sorry for the noise.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 24: 2611816-22.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs work » Needs review
FileSize
2611816-28.patch1.48 MB
interdiff.txt837 bytes

Fixed the remaining failure.

Status: Needs review » Needs work

The last submitted patch, 28: 2611816-28.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs work » Needs review
FileSize
2611816-30.patch1.48 MB
interdiff.txt2.31 KB

Expanded the test coverage of \Drupal\Tests\Component\DependencyInjection\Dumper\OptimizedPhpArrayDumperTest

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Title: [experiment] Update to symfony 3.0 » Update to symfony 2.8
Issue summary: View changes
Issue tags: -need work
FileSize
core.patch29.38 KB

First a patch for the core only changes

Note: We cannot yet update to symfony 3.0, due to
a) vendor/twig/twig/composer.json:34: "symfony/debug": "~2.7"
b) vendor/symfony-cmf/routing/composer.json:17: "symfony/routing": "~2.2",
c) vendor/stack/builder/composer.json:15: "symfony/http-kernel": "~2.1"

Status: Needs review » Needs work

The last submitted patch, 31: core.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs work » Needs review

Yeah that patch was not meant to be tested

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

@dawehner pointed out in irc that doing 2.8 in 8.1.x then 3.x in 8.2.x gives contrib modules a full release to clean up any deprecation notices, which makes sense to me.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
FileSize
2611816-35.patch1.48 MB
core.txt24.12 KB

Rerolled. Thank you @berdir for mentioning that!

Crell’s picture

Crell CreditAttribution: Crell at Palantir.net commented

By 8.2 we'd probably be looking at Symfony 3.1 or 3.2. Other than that note I think #34 makes sense and is consistent with what we've discussed for our own deprecated code elsewhere.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

#34 say 3.x, not 3.0, so not sure what you mean?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

Yeah we would go with ~3.0, whatever that is when 8.1 is out

Crell’s picture

Crell CreditAttribution: Crell at Palantir.net commented

I think we're all saying the same thing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

While checking when the first 3.x LTS would be, I noticed #2598662-28: Decide when Drupal 7 (and Drupal 8 and later) should enter the LTS and security-support-only phase.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

Are we waiting for something on this issue actually? Just being curious, we could be in something like "code freeze" for 8.1.x still?

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
FileSize
interdiff-35-42.txt715 bytes
2611816-42.patch1.48 MB

Fixing a really small typo. I think 'provider' was a typo and I was going to change it to 'provide', but I think 'define' is a better word here. Thoughts?

+    public function testGetListenerPriority()
+    {
+        // Override the parent test as our implementation doesn't provider
+        // getListenerPriority().
+    }
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

@dawehner for me this is an issue which could go into 8.1.x whenever, no need to wait.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

Fixing a really small typo. I think 'provider' was a typo and I was going to change it to 'provide', but I think 'define' is a better word here. Thoughts?

Well, this is a copy from symfony itself.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented

I knew other parts were a copy. I didn't know the test too was a copy. I am not sure what to do. For sake of consistency, we could just let it be the same as typo...

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

I knew other parts were a copy. I didn't know the test too was a copy. I am not sure what to do. For sake of consistency, we could just let it be the same as typo...

I just could not care less about that bit :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
FileSize
2611816-47.patch24.13 KB

Rerolled against no vendor dir. Its so much better

Status: Needs review » Needs work

The last submitted patch, 47: 2611816-47.patch, failed testing.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented

There is probably something wrong with how composer is run.

19:22:18 Executing setup:composer
19:22:18 Loading composer repositories with package information
19:22:18 Installing dependencies (including require-dev) from lock file
19:22:18 Nothing to install or update
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs work » Needs review
FileSize
2611816-50.patch1.56 MB

Oh I somehow assumed that we got rid of the vendor dir already. Here is a patch with it.

Status: Needs review » Needs work

The last submitted patch, 50: 2611816-50.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs work » Needs review
FileSize
2611816-52.patch809.56 KB

Another try.

Status: Needs review » Needs work

The last submitted patch, 52: 2611816-52.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs work » Needs review
FileSize
2611816-54.patch809.56 KB
2611816-54.patch809.56 KB

Mhh, maybe --binary helps

The last submitted patch, 54: 2611816-54.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 54: 2611816-54.patch, failed testing.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

Just a heads-up that some issues with YAML are fixed in 2.8: #2591827: Add YAML linting to core coding standards checks

The last submitted patch, 47: 2611816-47.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs work » Needs review
FileSize
2611816-59.patch24.22 KB

Reroll after #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
FileSize
interdiff.txt483 bytes
2611816-60.patch24.22 KB

Stupid typo.

The last submitted patch, 59: 2611816-59.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 60: 2611816-60.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs work » Needs review
FileSize
2611816-63.patch776.53 KB

Rerolled

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
FileSize
2611816-core-only-do-not-test.patch60.8 KB

Here is core only patch for review.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community

It just has cloned vendor code updates so seems ready to me.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs re-roll, +Novice

This could do with some more explanation.

+++ b/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php
@@ -30,6 +34,146 @@ public function __construct(ParameterBagInterface $parameterBag = NULL) {
+      // @this line is changed.

Something like:

 // Suppress deprecation warnings when a service is marked as 'deprecated: %service_id%-no-warning'

Also needs a change notice.

Tagging needs re-roll/novice for those

Otherwise looks great to me and would be good to get in asap.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs work » Needs review
FileSize
2611816-67.patch776.66 KB
interdiff.txt2.12 KB
2611816-review.patch61.59 KB

Added a change record... and fixed that one comment.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community

would be good to get in asap.

/me RTBC immediately.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/lib/Drupal/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumper.php
@@ -252,6 +252,10 @@ protected function getServiceDefinition(Definition $definition) {
+    if ($definition->isShared() === FALSE) {
+      $service['shared'] = $definition->isShared();
+    }

@alexpott pointed out this looks bit odd, and I agree.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

Can't agree more.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
FileSize
2611816-67.patch775.79 KB

Just trying without that hunk. #69 reversed is the interdiff...

Status: Needs review » Needs work

The last submitted patch, 71: 2611816-67.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented 
+    if ($definition->isShared() === FALSE) {
+      $service['shared'] = $definition->isShared();
+    }

Well, reverting is wrong, but maybe $service['shared'] = $definition->isShared() is enough. I mean to be clear, what actually matters is the non shared case, as this is the non default behaviour.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Needs work » Needs review
FileSize
2611816-74.patch776.6 KB
interdiff.txt837 bytes

Yes that makes sense, just wanted to be sure it was absolutely necessary at all and not cruft.

Just with that change.

Status: Needs review » Needs work

The last submitted patch, 74: 2611816-74.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

OH yeah, now I even remember why I made exactly that change. Setting the shared flag always a) increases the stored container and b) requires a lot of changes in the test coverage of the optimized container. I could just not be bothered to change them.

Ankit Agrawal’s picture

Ankit Agrawal CreditAttribution: Ankit Agrawal as a volunteer commented
Assigned: Unassigned » Ankit Agrawal
Ankit Agrawal’s picture

Ankit Agrawal CreditAttribution: Ankit Agrawal as a volunteer commented
Status: Needs work » Needs review
Issue tags: +drupalconasia2016
FileSize
2611816-78.patch784.8 KB
interdiff-2611816-67-78.txt936 bytes

Re-rolled the patch with above changes. Please review. Thanks

Status: Needs review » Needs work

The last submitted patch, 78: 2611816-78.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs work » Needs review
FileSize
2611816-80.patch777.04 KB
interdiff.txt1.72 KB

IMHO exposing shared all the time is still wrong. This is optimized, so we try to minimize the size of each entry
I'll revert that for now and just expand the test coverage

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Issue summary: View changes
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Needs review » Reviewed & tested by the community

The comment in #81 is probably all we needed there, thanks.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x, thanks! Great that this got in with a bit of time to spare before the 8.1.0 beta.

  • catch committed df9a828 on 8.1.x 
    Issue #2611816 by dawehner, jibran, catch, hussainweb, Ankit Agrawal:...
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

Nice!

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito at Lingotek commented

Created #2674780: Unexpected API change in UrlGenerator::generate()

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Status: Fixed » Needs work 
+++ b/core/core.services.yml
@@ -497,6 +497,7 @@ services:
+    deprecated: %service_id%-no-warning

Invalid Yaml

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Status: Needs work » Needs review
FileSize
update_to_symfony_2_8-2611816-88-quickfix.patch482 bytes

This fixes it. The only way I have to suggest confirming it is pasting the yaml here: http://www.yamllint.com/ (sorry its slow) or grabbing the patch in #1920902-268: Add a Drupal Yaml wrapper so we can default to PECL Yaml component if it is available and running the phpunit unit tests with pecl yaml installed.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler at erdfisch commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs re-roll, -Novice

Looks good to me.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

+1

  • catch committed 69901df on 8.1.x 
    Issue #2611816 by dawehner, jibran, catch, hussainweb, Ankit Agrawal,...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x, thanks!

Status: Fixed » Closed (fixed)

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

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue tags: +8.1.0 release notes
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented

Created #2703319: Update to symfony/css-selector 2.8 as follow up and there is a new one #2702677: Update to Symfony 2.8.4.