Comments

tim.plunkett created an issue. See original summary.

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

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tim.plunkett’s picture

Initially I thought this should go ahead and completely merge layout_discovery into system.module where it belongs. Not sure if that's still the plan.

Only removing @internal is pointless if the service definition still lives in an experimental module. So either we make layout_discovery non-experimental, or we move everything to system.module.

Here's one of each.

Both are blocked by #2852608: Review layout CSS and markup, but might as well get the discussion rolling.

tim.plunkett’s picture

Issue tags: +Blocks-Layouts
swentel’s picture

Given that DS and panels (and maybe others already too ?) have branches depending on layout discovery, the least disrupting way would just be to mark layout discovery as stable I think.

Also, give this:

+++ b/core/modules/system/system.layouts.yml
@@ -72,7 +72,7 @@ layout_threecol_33_34_33:
-  library: layout_discovery/threecol_33_34_33
+  library: system/drupal.layout.threecol_33_34_33

That would need an upgrade path no ? For DS, panels, Field layout, others to change the library ? Every module should have this then which becomes a bit tedious, I think.

So, I'm +1 on simply marking layout discovery as stable (but I'm fine with either decision).

tim.plunkett’s picture

That change wouldn't require an upgrade path, no one else should be addressing those libraries directly.
The key thing is that the plugin IDs do not change.

I'm also leaning toward marking layout_discovery stable for at least one release.

swentel’s picture

Regarding library: good point, my fault. We should refactor this in DS because we store it (so we can unset it) .. will file a bug report for that :)

tim.plunkett’s picture

The final blocker is in.

tim.plunkett’s picture

Title: Remove @internal from Layout classes when stable » Mark Layout Discovery as a stable module
Issue tags: +Needs release manager review

Guessing this needs RM review since it is about moving out of experimental

Wim Leers’s picture

Interesting, this is both:

  1. marking APIs as non-@internal
  2. marking the module as stable (non-experimental)

But there are still a lot of open issues: 1 bug, 12 tasks, 3 feature requests at the moment. So that means you're confident that completing those tasks won't break any of the APIs? Things like #2834023: Discuss service ID and config schema naming for Layouts, #2846395: Increase module weight of field_layout, #2846393: Investigate alternative approaches to moving fields in FieldLayoutBuilder::buildView() and #2796877: Devise a mechanism for mapping old regions to new ones when moving between layouts look (at first sight) that they might cause or require BC breaks?

I'm asking this with the best intentions: I'd love to see more experimental modules become stable, but in being asked to maintain the REST module, I've seen the negative consequences of marking something as stable prematurely. Repeating those mistakes will cost us.

tim.plunkett’s picture

Of those, only #2834023: Discuss service ID and config schema naming for Layouts would be a BC break, and I just closed it.
Most of those other issues are specifically for Field Layout module, not the API.

tim.plunkett’s picture

Priority: Normal » Major
Issue tags: +blocker
tim.plunkett’s picture

tim.plunkett’s picture

Status: Postponed » Needs review
FileSize
12.54 KB

Reroll now that those two are in.

DamienMcKenna’s picture

Should the HTML changes be separated from the API documentation changes?

xjm’s picture

Thanks @DamienMcKenna. In most cases, I'd say yes, but in this issue, both have to happen at the same time because both are required to mark the module stable, while neither can happen before the module is stable. :) In this case, the new templates are just copies of the templates that the module already provides (and have been reviewed already in #2852608: Review layout CSS and markup).

xjm’s picture

Is https://www.drupal.org/node/2619128 complete and up to date? That's docs gate for this.

xjm’s picture

Status: Needs review » Needs work

@tim.plunkett pointed me to https://www.drupal.org/docs/8/api/layout-api which is the correct handbook page, so we need to update the hook_help() for this as well.

Manuel Garcia’s picture

Status: Needs work » Needs review
FileSize
1.11 KB
13.65 KB

Bright future for layouts in Drupal, can't wait!

Updating the documentation link on hook_help (#18).

swentel’s picture

+1 one from me!

japerry’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to panels! +1

tim.plunkett’s picture

For the record, #20 is signoff from a Display Suite maintainer, and #21 is Panels/Panelizer.

Dries’s picture

+1 from me! Thanks everyone for all the hard work on this.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 19: 2834025-layout-19.patch, failed testing. View results

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community

  • xjm committed f9e40fa on 8.5.x
    Issue #2834025 by tim.plunkett, Manuel Garcia, swentel, japerry: Mark...
xjm’s picture

Status: Reviewed & tested by the community » Fixed

Thanks @swentel and @japerry!

Considering this for stable core inclusion:

  • We have all the needed signoffs, including from Dries.
  • The frontend and backened architecture has been reviewed and is stable.
  • It meets the core testing, documentation, and performance gates.
  • The accessibility and user facing gates are not relevant since the module does not provide user-facing functionality.
  • Most of the "should-have" issues from the roadmap are complete.
  • It's best for the ecosystem if this module is stable in 8.4.x and we only make BC additions in the future, since Panels already depends on it and is stable.

Based on all that, committed and pushed to 8.5.x and 8.4.x! Yay.

  • xjm committed 263d6cd on 8.4.x
    Issue #2834025 by tim.plunkett, Manuel Garcia, swentel, japerry: Mark...
xjm’s picture

Issue tags: +8.4.0 release notes
DamienMcKenna’s picture

Thank you one and all!

Wim Leers’s picture

👏🎉

markusa’s picture

This is fantastic for the project to have this. You'll have done an excellent job. Congratulations