YamlFileLoader was copied from Symfony 2.3. We updated to 2.4, but missed the literal copy.

I investigated whether it is possible to extend Symfony's instead of copying, but no luck:

https://github.com/symfony/symfony/pull/10920

For now, let's just simply update the code with latest upstream code.

Note that this intentionally violates Drupal coding standards. The code is a literal copy from upstream.

CommentFileSizeAuthor
#3 di.yamlfileloader.3.patch17.64 KBsun
drupal8.yamlfileloader.0.patch17.49 KBsun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

drupal8.yamlfileloader.0.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal8.yamlfileloader.0.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Needs review
FileSize
di.yamlfileloader.3.patch17.64 KB

Merged 8.x.

znerol’s picture

znerol CreditAttribution: znerol commented
Status: Needs review » Reviewed & tested by the community

Reading the patch and also the pull-request, this is indeed an unfortunate situation. Would it help when the meat of the class would be extracted into a trait upstream? It looks to me that we just might want to reuse parseDefinition[s]() and resolveServices().

Other than that the patch looks ok.

An idea for a follow-up: Perhaps it would be possible to extract the static caching into a wrapper class in order to further align the actual YamlFileLoader with upstream. Not sure how much that helps though.

  • Commit bfd7420 on 8.x by catch: 
    Issue #2269385 by sun: Fixed DependencyInjection YamlFileLoader is out...
catch’s picture

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

This is irritating but I don't have better suggestions atm. Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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

znerol’s picture

znerol CreditAttribution: znerol commented

Opened #2375339: Update DependencyInjection YamlFileLoader for Symfony 2.6