Note: this policy is now in the Drupal.org handbook and further updates will be made there.

Now that Drupal 8-beta 1 has been released. it’s important for all contributors to core to understand the criteria for acceptance of core patches during this phase of the development cycle.

This proposal is based on discussions between @Dries, @catch, @alexpott, @webchick, @effulgentsia, and @xjm, with input from numerous other core contributors (thanks everyone!).

Policy goals

  • Reduce technical debt and focus on necessary work to complete Drupal 8.0.0.
  • Set guidelines to help contributors avoid focusing energy on issues which won’t be committed until 8.1.x or 9.0.x, since it can be frustrating to get a patch to RTBC only to find it won’t be committed until 6-48 months in the future.
  • Avoid moving issues to 8.1.x or 9.0.x prematurely when they're still acceptable and valuable during this phase of the release cycle.
  • Reduce the need for contributors to consult core maintainers when contributing to the beta phase.
  • Provide example issues with the reasoning for when and whether each can be accepted.

How to evaluate issues (#)

  1. Assess and correct the issue category and issue priority. Make sure only actual feature requests are marked as such; make sure only actual bugs are marked as such (i.e. a failing test can be written); set the priority correctly. Maintainers have final discretion on issue priorities.
  2. Assess whether it is a change only to one or more unfrozen categories (see below): CSS, markup, translatable strings, documentation, automated tests.
  3. Assess whether the primary purpose of the change is a prioritized change (see below): performance, security, usability, accessibility, bug fix, or a followup to a recent critical or major.
  4. Assess whether the issue reduces fragility. Examples: narrowing an API, removing unused and untested functionality, correcting misspelled or wholly inaccurate method names. Whether or not an issue reduces fragility is at core maintainer discretion and should be discussed on a case-by-case basis.
  5. Assess whether the impact is greater than the disruption (see below). If the summary doesn't communicate the impact or the disruption, leave a note on the issue asking the contributors to update it. Whether or not the impact is greater than the disruption is at core maintainer discretion and should be evaluated on a case-by-case basis.
  6. Ensure the above information is clear from the issue summary (see below).

Flowchart (#)

Flowchart of how to assess changes during the D8 beta

What changes are "unfrozen" (1)? (#)

Improvements can be accepted up until the first release candidate if they only change the following:

  • CSS
  • markup
  • translatable strings
  • documentation
  • automated tests

Additionally, the provision of a Drupal 6/7 to Drupal 8 migration path is not strictly coupled to the release cycle, so that code remains unfrozen.

What changes are "prioritized" (2)? (#)

The following kinds of changes are prioritized because they improve Drupal 8's stability or move Drupal 8 closer to a releasable state:

  • bug fixes
  • external PHP and asset library updates
  • usability and user experience improvements
  • accessibility improvements
  • performance improvements
  • security improvements
  • follow-ups from a recent critical or major change
  • code already marked for removal by 8.0.0

Additionally, certain changes that reduce fragility are also prioritized. For example: Narrowing an API, removing unused and untested functionality, correcting misspelled or wholly inaccurate method names. Whether or not an issue reduces fragility is at core maintainer discretion and should be discussed on a case-by-case basis.

Is the change disruptive? (#)

A change is disruptive if it:

  • Introduces a BC break that will affect many contributed modules, or require some contributed modules to make non-trivial changes
  • Will require internal refactoring or rewriting of core subsystems, as these changes tend to introduce technical debt and regressions
  • Will require widespread documentation or code style updates which are likely to conflict with other patches in the queue
  • Will require changes across many subsystems and patches in order to make core consistent.

Do I need a BC layer? (#)

For allowed changes that would introduce a backward compatibility break, a BC layer is needed unless:

  • a BC layer is not feasible,
  • providing a BC layer would introduce significant technical debt, or
  • the API is sufficiently new that it is unlikely to be used anywhere.

What does it mean for the change to be postponed? (#)

Changes that are no longer accepted during the Drupal 8 beta phase will be allowed again in either a minor version (8.1.x) or the next major version (9.0.x).

Minor version target (e.g. Drupal 8.1.x)

New features, API additions, or internal refactoring that can be implemented in a non-BC-breaking way, or with a BC layer.

Drupal 9.0.x

Major subsystem rewrites: for example, refactoring the render API to use object oriented programming techniques. Removing backwards compatibility layers added during the beta and in minor releases.

Issue summaries matter (#)

It's important to have a good ">issue summary so branch maintainers can quickly understand each change and make fair decisions about it. With the definitions above in mind, be sure to describe:

If your issue summary is unclear or not up to date, branch maintainers may mark your issue "Needs work" for an issue summary update.

Appendix: Some definitions

Drupal 8 branch maintainers
The Drupal 8 branch maintainers are the four core committers with the authority to commit functional code changes to the 8.0.x branch of core: catch, alexpott, webchick, and Dries. (Other core subsystem maintainers and topic coordinators in MAINTAINERS.txt are not branch maintainers)
Issue category
See the category settings for issues for guidelines on what is a bug, task, or feature.
Issue priority
See the priority settings of issues for explanation of what is critical, major, normal, or minor. Branch maintainers have final discretion over each issue's priority, but use your best judgment to set it accurately now.
Backward-compatibility (BC) break
A backward-compatibility break is a type of API change that requires changes in code implementing a public API. This can be anything from changing the parameters of a procedural function to renaming an interface or removing a hook.
Backward-compatibility layer
Many API changes can provide a backward compatibility layer that allows code using the old API to continue to work. For example, if a method has a badly misleading name, a BC layer can be preserved by leaving the badly-named method as a wrapper for the new one and marking it deprecated.
Minor version (e.g. 8.1.x)
Drupal 8 introduces a six-month minor release cycle. Minor releases provide new improvements and functionality without breaking BC for public APIs. See the proposal to manage the Drupal 8 release cycle.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes

Also note that data model/schema changes are covered by this issue: #2341575: [meta] Provide a beta to beta/rc upgrade path

Note that contributed project blockers (that is, changes that completely prevent a contributed project's upgrade to Drupal 8) are generally critical. Fixes that help contributed modules but do have workarounds (even if they are undesirable or have bad DX) are generally major.

xjm’s picture

Issue summary: View changes
Bojhan’s picture

Issue summary: View changes

I am up for helping visualise this, is it possible to get some pointers during todays sprint.

I do have a few comments coming from Drupal 7 release management were we established a lot of practices.

"What the impacts of making the change are for both core and contributed modules."

  • It would be extremely valuable to have a few examples of this. If it linked to precedent that its even more useful.
  • Could we consider adding this to the default Core issue template?

"Improvements can be accepted for the following if the issue is critical, or..."

  • This seems awfully strong, and quite new to me. Is this agreed upon by all committers? As far as I know, we should continue to improve Drupal even on non-critical issues. There are hundreds of patches (feature, bug, task, etc) that wont negatively impact contrib or core. The main requirement being is that it does not introduce any disruptions, regressions or backlog. If you disallow all non critical/major patches, you are eliminating a extremely large part of our contributor base to core (basically, I cant work on core anymore for atleast year).
  • I suggest to follow the; no disruptions, backlog or regressions model and apply that to everything. With a highly skewed focus on fixing criticals/majors but not disallowing anything else. The whole idea of this phase is to allow polishing our parts (we didn't really have a proper polish phase).

For component maintainers that are a lot of things we can do:

  • Feature requests shouldn't introduce any backlog, regressions or disruptions. While backlog is often unavoidable, at best it should be followups with very little impact.
  • Bug fixes should be highly actionable and contained. More manual testing and a longer timespan to make sure we don't introduce any new issues.
  • Critical or majors, should only introduce issue(s) (dependancies) that are manageable before RC1 - its often tempting to exchange one critical for a major. Thats why 7.1 was so fundamental for Drupal 7, because we fixed issues that were kinda critical.
  • Educate all your contributors, to get in the mindset of releasing Drupal. This means that they work on the biggest issues in your component, and if possible cross to others were there still are issues. Don't introduce "new" issues. Help them understand, why the quality of a patch now needs to be higher.

The text is a little hard to follow, because the titles are so similar (frozen, unfronzen, mostly unfrozen).

tstoeckler’s picture

Issue summary: View changes
Issue tags: +Amsterdam2014

So we have the following situation with #2230637: Create a Language field widget and the related formatter:
Language module currently has a configuration called

Show language selector on create and edit pages

with the configuration key language_show. We realized that this is really an access-related setting and should be applied e.g. during content creation via REST, as well. This would not be an API change in itself, but since are changing the semantics of that setting to be purely UI related the name language_show would be a very bad name. I.e. you would have to configure it when using REST which is not very intuitive.

The issue itself is a critical data integrity bug, but it can be solved without renaming the config name. The question is just whether the criticality also would allow us to change the config name, in order to avoid a really big WTF.

YesCT’s picture

Issue summary: View changes

I was confused when I read https://www.drupal.org/core/issue-priority
We need to clarify it, specifically adding task descriptions (and examples) for normal and minor.

And rephrase an item for critical that clarifies removal of a BC layer... means like an entire huge system (not the regular old removal of a deprecated method)

Since that issue-priority page should really only be changed by maintainer agreement, commenting here.

xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes

This seems awfully strong, and quite new to me. Is this agreed upon by all committers? As far as I know, we should continue to improve Drupal even on non-critical issues. There are hundreds of patches (feature, bug, task, etc) that wont negatively impact contrib or core. The main requirement being is that it does not introduce any disruptions, regressions or backlog. If you disallow all non critical/major patches, you are eliminating a extremely large part of our contributor base to core (basically, I cant work on core anymore for atleast year).

The key thing in the proposal that we are not disallowing them in general -- we are only disallowing them if the disruption is greater than the impact/value (edited for typo). :) For the cases you describe, presumably, the disruption would be very low and therefore they would be accepted.

catch’s picture

Issue summary: View changes
YesCT’s picture

xjm’s picture

Issue summary: View changes
Bojhan’s picture

So I propose to revamp this a bit, because the whole (frozen, unfrozen, mostly frozen) part really confuses me. There are a few things we can optimize:

- Clearly distinguish the requirements to meet on top
- Separate out advise how to handle "non applicable" changes from the advice on what is allowed. So separating "qualifications" from "how to handle it in the queue".

My lullapad is here, it has quite an impact so I didn't update the sumaary (as advised by xjm). Part of this is the fact that our titles are not different enough but its also organisation.
https://pad.lullabot.com/p/i2ZYa2g6MR

almaudoh’s picture

Issue summary: View changes

...Oct. 27 (which will be the commit freeze prior to the tagging of 8.0.0-beta1)...

This should be a typo right? beta1 is already tagged. I took the liberty to change it to beta2...pls correct me if I'm wrong.

xjm’s picture

Issue summary: View changes

Yep @almaudoh, thanks!

Removing a specific point about coding standards issues based on discussion with @alexpott @effulgentsia and @catch since we can't guarantee the "late beta" window for them.

benjy’s picture

Great write up @xjm, thanks.

I know it's understood amongst most contributors but it would be nice if this policy mentioned that "new features" or non-critical migrate patches will still be committed during beta as not to confuse or discourage new contributors from working on migrate patches?

catch’s picture

Issue summary: View changes

@benjy I updated the issue summary for migrate - does that look OK?

benjy’s picture

@catch, yeah that's great. Thank you.

catch’s picture

Issue summary: View changes
catch’s picture

The backwards compatibility section of this is hard to apply consistently. Discussed with Alex and we think the following might be OK.

This should only apply where we think there's a decent chance of contributed modules getting affected, not for dead code. Also we can reserve the ability to break modules early and explicitly due to a critical issue. Additionally it only really applies for new deprecations of functions that weren't previously.

The idea we came up with is to support contributed/custom modules that are porting against each beta release.

What we should try to avoid outside of exceptional circumstances:

Module ports to beta 1, function is not deprecated.

Module ports to beta 2, function is removed, BOOM!

The wording would be like this. This assumes we are currently at beta 1, the patch goes in before beta 2 is cut.

@deprecated in Drupal 8.0.x-dev, will be removed no earlier than Drupal-8.0.0-beta3 and before 9.0.0 use blah instead

This means:

Module ports to beta 1, function is not deprecated.

Module ports to beta 2, function is deprecated - has ability to update without fatal errors.

Module ports to beta 3 (or later), function is removed, already updated so doesn't care.

webchick’s picture

I worry that direction could lead to more "nice to have" refactoring and not more getting the release out the door sooner, which is what we need to be focused on at this stage. It also (at least potentially) leads to us needing at least one extra beta we wouldn't have otherwise have needed in order to give people the opportunity to see a function's deprecated before RC. (EDIT: And also creates issue management headaches, because you need to mark an issue fixed, then... postponed? Then somehow manage to find them later to remove the dead code in time. More time/energy/attention/focus diverted away from getting D8 out the door. :\)

If we didn't manage to mark a function deprecated in 3.5 years, why can it not live until D9?

webchick’s picture

"If we didn't manage to mark a function deprecated in 3.5 years, why can it not live until D9?"

That's just for the function itself, btw, as a BC wrapper. In 8.X.x we could happily be removing procedural cruft from the core code base, marking things deprecated, and converting all of core's code to use OO goodness instead, and recommending such in the release notes. I just feel strongly we need to stop doing that now, when we're staring down an ever-growing pile of critical bugs standing between us and D8's release.

Bojhan’s picture

So I proposed a rewrite to make things more clear, I am not sure if thats still being considered or if my work there was moot.

@catch This is not usually something I comment on at all, and was very hesitant to do so. But I am very worried about all the strategies that seem to impact our ability to release. I don't feel like there is the same urgency, to release Drupal 8. While we don't have much market analysis, I feel like much of our momentum is flattening - having Drupal 8 out the door is monumental and key to our larger ecosystem.

Rules should be evaluated whether it helps us getting closer to a quality release. I don't feel that there is the same urgency amongst comitters, and that is worrisome. We already communicated to people that during beta, we might change critical things - contrib maintainers are aware of this and know this from our previous releases. We already communicate very clearly that removing/deprecating API's should by large apply only to major/critical issues. I feel like most of the management in between beta releases, has very limited value in the large scheme of things - and ideally we wish to compel those contrib maintainers to be somewhat aware of core, and ideally even move between the contrib and core queue.

YesCT’s picture

@Bojhan I would suggest making your changes directly in the summary here... it makes a nice diff people can review, if they feel like it changed meaning (instead of just making things more clear). I say this, because I thought that your re-write got feedback from xjm and others during AMS...

daffie’s picture

@catch I know you mean well with the removal of functions. But I think as a module developer do it like the removal of a band-aid: quick and fast!

daffie’s picture

Is it possible to get a list of function, per beta release, that where removed since the last beta release. That will make it more easy the solve bugs that are the result of the removed functions. Just an idea.

catch’s picture

@webchick I'm thinking of issues like #2350437: Mark unicode.inc functions as deprecated where the refactoring was done weeks/months ago but we just forgot to mark the wrappers deprecated.

Or #2350949: Add hook_page_attachments(_alter)() and deprecate hook_page_build/alter() which is critical, but where there's disagreement on the issue as to whether to leave a BC layer in at all or not (see Moshe's comment) and where it's not just a wrapper - in fact that's the main reason that critical issue isn't RTBC at the moment.

In both of those cases it's not extra refactoring, but stuff we would/have done anyway and handling the impact on modules.

With the last beta - if something gets marked deprecated then, we can just leave it deprecated until 9.0.x, no need to put out another beta.

I'm also fine with leaving things in if there's no good reason to remove them and the band aid approach that @daffie said, but since we'll need to remove some cruft, and deprecate some things, trying to come up with some consistent policy seemed better than having to individually communicate and discuss on every single issue that does this which course of action to take - which also takes energy away from getting the release out. Especially when it's critical issues that are either adding BC layers we don't want, or taking out BC wrappers we might want - lack of consistency makes it harder to work on/review/commit those issues and we can't just pretend we don't need to deal with it.

catch’s picture

Assigned: Unassigned » Dries
Status: Needs review » Reviewed & tested by the community

Moving this to RTBC - we can always open a side-issue to sort out deprecations further, but I think there's pretty broad agreement on the issue summary itself. Will need transferring to the handbook before we mark fixed. Also most of this was discussed with Dries in person before it got written up/refined but assigning since he hasn't commented here yet.

webchick’s picture

I guess it's fine, since all of the "insiders" have seen it (well, 26 of them anyway), but one thing we never did was inform the wider community of this proposal. A post to g.d.o/core was blocked on a diagram and example issues, which are still listed as "forthcoming."

I guess we can inform the wider community after the fact of this being approved, but I have doubts that all of the people we want to have seen this proposal actually have seen it given the types of issues I'm still seeing being worked on right now.

catch’s picture

There are example issues in #12.

#2070737: Change values of LanguageInterface::DIRECTION_(LTR/RTL) to ('ltr'/'rtl') also has some discussion. That might be enough to write some things up, not sure.

I'd be OK with a g.d.o/core post while this is RTBC for final comment, but diagrams made by me are unlikely to make this clearer and we're already applying the policy to patches - so the longer it's in an issue and not in the handbook/g.d.o/core the more confused people who haven't seen it will continue to be. We really should have had this written up before the beta was tagged, but that didn't happen.

Bojhan’s picture

Status: Reviewed & tested by the community » Needs work

I am really not sure about the way this information is presentable. Its quite hard to grok. I'd love if we take another stab at visualising and rewriting this.

Compared to the previous documentation we had for Drupal 7 its cycles, this is far less usable for new people to understand what is going on. Webchick her blogposts were always quite useful in understanding. Lets see if we can learn from that previous communication.
- My rewrite: https://pad.lullabot.com/p/i2ZYa2g6MR
- I am happy to help visualising, but frankly not sure if I should input more work, considering the fact that my lullapad wasn't looked at.

Marking this tentative, to needs work. Since this mainly about the "how" not the "what" of communicating this.

catch’s picture

@Bojhan I didn't know there was a lullapad :( Looking now.

catch’s picture

Issue summary: View changes

I've moved the lullapad to the issue summary, that's much clearer.

daffie’s picture

All the stuff that is in the issue summary is all very theoretical. What I would like to see is what that means for the individual issues out there. Can the four core committers decide for the individual issues whether they are going to be included in Drupal 8.0.0 or that the issue is going to be postponed until Drupal 8.1.0 or 9.0.0. I know this will make a lot of core developers not very happy. But at the moment there a lot of issues being worked on that are for instance beta-target. And I think that if we want a speedy release of Drupal 8.0.0 that some or a lot of those issues need to be postponed until after the release of Drupal 8.0.0.

catch’s picture

Status: Needs work » Needs review

Can the four core committers decide for the individual issues whether they are going to be included in Drupal 8.0.0 or that the issue is going to be postponed until Drupal 8.1.0 or 9.0.0.

No, there are five core committers, and over 4,000 open issues against 8.0.x. The point of the issue summary is to give a framework so that people can figure out themselves whether an issue is eligible or not.

Since the issue summary now contain's Bojhan's rewrite, moving back to CNR.

daffie’s picture

For most of the issues it is clear if they are allowed for Drupal 8.0.0. But there are a few issues that can be argued pro and con. If the five core committers can decide those issues then all other issues will clear what their status will be. It will make this policy concrete and clear to everybody. So that we all can concentrate and work to releasing 8.0.0.

catch’s picture

@daffie issues that are borderline and people can't figure out themselves, they can ping one of us for feedback - but see the notes regarding issue summaries etc.

Bojhan’s picture

Issue summary: View changes
catch’s picture

Component: base system » documentation
xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes

So we discussed today how to make this simpler/clearer since the summary now includes two wholly different drafts of the same information. :) Working on that. Meanwhile, we agreed on a higher-level TLDR for the questions contributors should consider when evaluating issues during the beta:

  1. Assess and correct the issue category and issue priority. Make sure only actual feature requests are marked as such; make sure only actual bugs are marked as such (i.e. a failing test can be written); set the priority correctly. Maintainers have final discretion on issue priorities.
  2. Assess whether it is a change only to one or more unfrozen categories (see below): CSS, markup, translatable strings, documentation, automated tests.
  3. Assess whether the primary purpose of the change is prioritized change: performance, security, usability, accessibility, bug fix, or a followup to a recent critical or major.
  4. Assess whether the issue reduces fragility. Examples: narrowing an API, removing unused and untested functionality, correcting misspelled or wholly inaccurate method names. Whether or not an issue reduces fragility is at core maintainer discretion and should be discussed on a case-by-case basis.
  5. Assess whether the impact is greater than the disruption (see below). If the summary doesn't communicate the impact or the disruption, leave a note on the issue asking the contributors to update it. Whether or not the impact is greater than the disruption is at core maintainer discretion and should be evaluated on a case-by-case basis.
  6. Ensure the above information is clear from the issue summary.
xjm’s picture

Here is an outline for a flowchart:

  1. Assess and correct the issue category and issue priority.
    • If the issue is a critical bug or critical task, GREEN LIGHT (proceed with issue).
    • If the issue is a feature request, RED LIGHT (postpone).
    • Otherwise, go to #2.
  2. Assess whether it is a change only to one or more unfrozen categories (see below): CSS, markup, translatable strings, documentation, automated tests.
    • If the issue is in one of these categories, GREEN LIGHT (proceed with issue).
    • If the issue is correctly marked as a major bug, major task, or normal bug, go to #5.
    • If the issue is correctly marked as a normal task, go to #3.
  3. Assess whether the primary purpose of the change is prioritized change: performance, security, usability, accessibility, bug fix, or a followup to a recent critical or major.
    • If yes, go to #5.
    • If no, go to #4.
  4. Assess whether the issue reduces fragility.
    • If yes, go to #5.
    • If no, RED LIGHT (postpone).
  5. Assess whether the impact is greater than the disruption (see below).
    • If yes, GREEN LIGHT (proceed with issue).
    • If no, RED LIGHT (postpone).
xjm’s picture

And a flowchart.
Flowchart of how to assess issues during the D8 beta

P.S.: Thanks to @alexpott for catching a couple bugs in the chart.

xjm’s picture

Issue summary: View changes

Removing duplicated and outdated text from the summary.

xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes
alexpott’s picture

@xjm Thanks for the new flowchart - super clear and easy to grok.

We need to discuss removal of deprecated functions. Eg.

Those are currently the oldest 3 issues in the rtbc queue. And they are all normal tasks - which I think is the correct status.

Considering that these patches are removing usages of functions that we've deprecated in previous patches I think that we should be committing these patches under the aegis of either a new category of prioritised changes or under "follow to a recent critical or major". Unfortunately we can not be sure that the function was deprecated by a critical or major or was actually that recent. So I think we should add a new category of "Removal of functions and methods that we have documented will be removed prior to 8.0.0".

If we do not want to do this the only alternative is to file an issue to change the deprecation of all the functions to 9.0.0 - which I think is silly. One reason is that this is a finite piece of work if we also agree that we will not (unless necessary) mark any more functions for removal prior to 8.0.0 - I think we have already tacitly agreed to this by calling the beta. Another reason is we've said we are going to do this and already agreed to it. Lastly, this issues remain a good entry point for contributors and many of the existing issues have work from people new to contribution so as a first experience it would suck to say well we said this then but now we're going to make you wait x years.

daffie’s picture

+1 For removing the deprecated functions. Module developers know or should know that these functions will be removed in 8.0.0. So lets remove them.

effulgentsia’s picture

So I think we should add a new category of "Removal of functions and methods that we have documented will be removed prior to 8.0.0".

+1

However, I think that category should still point to the "impact > disruption" box. Something like #2353013: Remove taxonomy_term_load_parents_all from taxonomy/taxonomy.module seems like an obvious yes to me, as it causes no disruption. However, #2361823: Remove usage of drupal_strtolower() changes ~100 hunks of code, so might require rerolling lots of other core patches, but that's mitigated by each such reroll being easy. So I could see something like that needing a judgment call by the committer.

alexpott’s picture

However, I think that category should still point to the "impact > disruption" box.

+1 and if we put it in the "prioritised change" list then it already does :)

xjm’s picture

Issue summary: View changes
FileSize
73.96 KB
59.22 KB

I mostly/tentatively agree with #49 through #52, so long as we pay attention to the impact vs. disruption (as both Alexes point out), and so long as we are clear that this is only the removal of code that is already marked for removal in 8.0.0 and that already has an actual, complete, working replacement in core. As of 10 months ago we had a lot of "wishful deprecations", but a lot of work has been done since to fix that, and I think @alexpott checked and found that there was only one such case remaining. I think it also falls under the net of reducing fragility, because removing one of two duplicate ways of doing something (one of which is probably untested) is going to help stabilize D8.

So I've added it to the summary and updated diagram (attached). The updated diagram includes some text footnotes for reference, based on feedback from @Gábor Hojsty and @alexpott.

xjm’s picture

Oh, and I believe at some point in the future we should actually switch to changing the deprecation markers to 9.0.0 if core doesn't already use the new API, by the first RC if not before.

xjm’s picture

Issue summary: View changes
effulgentsia’s picture

The issue summary looks great to me. Just a couple nits:

external PHP and asset library updates

This is in the summary text, but not in the diagram footnote.

If a change can be accepted based on the criteria above but introduces a backward compatibility break, the patch for the issue should also provide a backward compatibility layer if feasible and where the BC layer would not introduce significant technical debt, or where the API is sufficiently new that there is no likely chance of it having any consumers anywhere.

Awkward grammar: does this mean that if the API is sufficiently new, then a BC layer should be or should not be provided?

xjm’s picture

Issue summary: View changes
FileSize
74.93 KB
59.32 KB

I changed the BC text to:

For allowed changes that introduce a backward compatibility break, a BC layer is needed unless:

  • a BC layer is not feasible,
  • providing a BC layer would introduce significant technical debt, or
  • the API is sufficiently new that it is unlikely to be used anywhere.

(I think that's what that paragraph means, anyway.) ;)

Dries’s picture

Assigned: Dries » Unassigned
Status: Needs review » Fixed

This looks great to me! Excellent work, xjm and team. Approved.

xjm’s picture

Issue summary: View changes
xjm’s picture

xjm’s picture

Issue summary: View changes
FileSize
75.43 KB
59.59 KB

Adding Migrate to the "unfrozen" changes in the flowchart (as per summary and #17 - #18).

webchick’s picture

Status: Fixed » Reviewed & tested by the community

We need this in the handbook before calling it fixed, per #29.

catch’s picture

Status: Reviewed & tested by the community » Fixed
jhodgdon’s picture

What happened to the nice diagram being part of the issue summary?

YesCT’s picture

@jhodgdon I still see the nice diagram... maybe internet hiccup?

catch’s picture

I think we need to add changes to individual theme functions to 'unfrozen' - since that is:

1. Necessary to make markup changes (for anything that's not already a template)

2. By definition low-impact unless it's extremely commonly used

This came up for #1938918: Convert menu theme tables to table #type which I'm about to commit based on that (if we don't add it to unfrozen, I think that issue would still be OK on impact > disruption.

jhodgdon’s picture

yeah, flowchart is back, must have been a hiccup indeed! who knows.

xjm’s picture

Issue summary: View changes
xjm’s picture

Re: #66, that issue is actually a child of #2348381: [META-0 theme functions left] Convert/refactor core theme functions which is (correctly I believe) major. So I don't think we need to add a specific listing if we agree that the impact of that whole change set is greater than the implied disruption.

I also think that theme functions and templates generally were in our original brainstorm about what was unfrozen? but I don't see it in my original draft of this issue. Since we explicitly indicated the beta was not a good porting target for themers, I think it makes sense to be more permissive with themeable output generally.

Thoughts? Are there situations where altering theme functions or templates would be too disruptive to module code?

David_Rothstein’s picture

Status: Fixed » Needs review

It sounds like this should still be open for #66/#69 (or is that already resolved?).

I also just came across this stuff, and really like the diagram but I see two problems:

  1. There is no provision for issues that are backportable to Drupal 7. That means that any Drupal 7 feature requests (and possibly some non-features too) that aren't in Drupal 8 yet will be held up in waiting, potentially for a very long time. There are only two issues in this state currently but I would expect more as additional issues get recategorized for 8.x-1.x or labeled for backport properly.

    I think in practice, allowing these kinds of issues into Drupal 8.0.x will not cause delays in the Drupal 8 release (pretty much by definition they fall into the impact > disruption category) so I would argue for just accepting them and seeing how it goes.

    Suggestion: New orange box at the top, "Backportable to Drupal 7" => "Proceed"

  2. The "Postpone" wording is confusing. Some people are taking that literally (currently around 60 Drupal 8.1.x issues marked with the "Postponed" status) though in more cases they are not. I think there's no reason to mark an issue postponed if it's already moved to 8.x-1.x; usually there is nothing actually blocking someone from working on it (even if it won't be committed for a while). I think that's consistent with how this was always handled in the past also.

    Suggestion: Rename the "Postpone" bubble to "8.1.x or later", and clarify in docs that 8.1.x and 9.x issues don't need to be marked postponed

YesCT’s picture

The action we want people to take is to both move it to 8.1.x or 9.0.x *and* mark it postponed.
We do not want people working on these things.
Those branches are not open for development yet, and are available to choose in the issue drop down for a version for bookkeeping only.
This is a change from how releases were managed before.

This will help new people be able to find issues to work on that are relevant now, and more likely to get feedback and reviews. (8.1.x and 9.0.x are not those. some of them have work already, and have lots of comments, so without the clear postponed on them, people may not realize that they should not be worked on right now)

https://www.drupal.org/contributor-tasks/update-allowed-beta
lists those steps also (change the version and mark it postponed, and add a sentence as to what it is postponed on: the opening of 8.1.x for development). If I am wrong, and we want to non "postpone" the issues... then that contributor task doc should also be updated.

daffie’s picture

As a contrib module developer I use drupal core as a reference how problems are solved. I follow the issue queue to what is still to be solved with a different solution. After an issue is landed I can check if I need to update my module. If open issues that I follow for my contrib module are changed to postponed to 8.1.x or 9.0.x, I know what the drupal solution is for a particular problem. I know that that does not mean that is the best solution. And I try to understand what the issue is about and why the drupal community chose the solution that it chose. And how that will effect my contrib module. But still for me and a lot of other (contrib) developers drupal core is the reference solution for the way (contrib) modules should be programmed.

YesCT’s picture

So, I read #16, but where does that leave coding standards?
I know for things like "get X module/component to pass coder review" we are postponing those (not on 8.1.x or 9.0.x, but on having our testbot automatically test for things). See #1518116-86: [meta] Make Core pass Coder Review

But what about more scoped things like the children under #2052421: [META] Rename Views properties to core standards like #2078593: In WizardPluginBase Rename Views properties to core standards are those also postponed under not meeting the criteria here and/or postponed on automatic tests for them?

David_Rothstein’s picture

The action we want people to take is to both move it to 8.1.x or 9.0.x *and* mark it postponed.
We do not want people working on these things.
....
This is a change from how releases were managed before.

Interesting... I am wondering if this was discussed anywhere? (I don't see a specific discussion in this issue.)

I see the point, but the downside is that issues tend to get forgotten after being postponed (someone could certainly go through all 8.1.x issues and unpostpone them later, but doing that quickly in bulk would result in some being unpostponed that shouldn't be, since they might also be postponed for another specific reason - i.e. a dependency on another issue that isn't complete yet). I can see this causing problems later on, or at least causing a lot of work unless the issue summaries are really perfect.

Also, there are good reasons to work on patches sometimes even if they can't be committed until a future release (rerolls to keep them fresh and not get too out-of-date, bigger issues that might take some planning and even patching in advance, etc). To get the testbot to run on such an issue you would need to move it out of "postponed" status anyway, I think.

Ideally I would expect the "8.1.x" version should indicate to everyone that it's not immediately relevant and won't be committed until that branch is open (even if it reaches RTBC status way before then) though I understand how the word "postponed" at the top of the issue page makes that more clear.

xjm’s picture

Yeah, we discussed when discussing this policy in Amsterdam (though it never ended up here, sorry about that) that it was very easy to search for "8.x issues" and miss that a particular issue was against 8.1.x; e.g. it was even happening to branch maintainers. Plus many new contributors don't necessarily know or find release cycle information, and we really want to encourage work on those issues that are still 8.0.x material, so we are explicitly posptoning things set against the 8.1.x branch and will reopen them later. The issue queue communicates better to people that way. The number of issues that might actually still need to be postponed when 8.1.x opens should be small and easily corrected if they get activity. Much better to have accidental activity on a handful of issues later than on hundreds of issues now.

Re: #73, what I wrote in #1518116-86: [meta] Make Core pass Coder Review about covers it. We thought about making a special window for coding standards cleanups, but did not come to a conclusion on it and could not guarantee it. It might be that we allow them later on based on the testbot support being available, or it might be that we allow them as soon as 8.0.0 is released. The one thing we did agree on is that they need to go into both 8.1.x and 8.0.x if they are made after 8.1.x is opened to ensure the branches do not diverge over them. For now, they should be treated like any other minor "non-priority" task and be postponed. E.g. #2052421: [META] Rename Views properties to core standards is not only a coding standards cleanup but also includes API breaks, so I agree that those should be postponed (and the children should be minor).

Regarding backports, I think stuff that needs backport to D7 should fall under the "prioritized categories" already? Since Drupal 8 has been in feature freeze since Dec. 2012 and over feature issue thresholds for longer, I feel it's hard to make a case for either of https://www.drupal.org/project/issues/search/drupal?version[]=8.1.x-dev&.... Nominally coding standards cleanups could be considered backportable to D7 as well, and we certainly don't want efforts invested into that right now. So I would at least want to see the Needs backport tag removed from issues that aren't strategically important in the ways that bugfixes, accessibility improvements, etc. encouraged for D8 are. If we find an issue we would want in D7 that we wouldn't allow in D8 currently, we need to ask ourselves why. Edit: Perhaps "normal task tagged for backport to D7" is a category we should triage soon, like "normal tasks tagged Novice" is.

Regarding the labeling of the "Postpone" bubble, the explanation is in the text in the summary and on the policy page, which people need to read. IMO the flowchart can't contain all the information and folks should familiarize themselves with the policy to know whether to pick 8.1.x or 9.x. I guess making the red bubble say "8.1.x or later" could be less confusing in one way, but then I worry people might not take the time to recognize which changes should be moved to 9.x. And I do think the explicit "Postpone" status is helpful.

@David_Rothstein, thoughts?

Edit: #66 and #69 are already addressed in those issues.

jhodgdon’s picture

I agree with David. Marking an issue "postponed" and moving it to 8.1.x seems like overkill. People are going to have to get used to there being multiple 8.x. versions.

YesCT’s picture

regarding #1921152: META: Start providing tour tips for other core modules. (meta is a normal task, and so are the children)
Can we confirm task issues that add tours is allowed?
I think maybe under the list of unfrozen allowed changes?
They are documentation (like): most adding tour yml, maybe a tour test, and css (or adding a css class into code to be used by the css).
@larowlan mentioned in irc remembering that @alexpott said tours could be committed up till string freeze.

YesCT’s picture

and want to confirm that #2016679: Expand Entity Type interfaces to provide methods, protect the properties and all of its children should be postponed? (ug. I dont look forward to explaining to php people why we use public properties as an API instead of methods)

wait... in #2030645-31: Expand Menu with methods @alexpott reclassified it as a bug. phew.
will do that for the meta and the children.

alexpott’s picture

Re #77/#1921152: META: Start providing tour tips for other core modules. I think adding tours should be allowed since as YesCT has pointed out it is adding documentation (tour yml), CSS and a test all of which are currently allowed. Additionally adding tours is very low risk and will benefit users.

xjm’s picture

Yeah I'd say tours actually fall under the usability "prioritized change" category.

Re: #78 we discussed this issue specifically when drafting the policy; that was our example of "reducing fragility".

xjm’s picture

Ideally I would expect the "8.1.x" version should indicate to everyone that it's not immediately relevant and won't be committed until that branch is open (even if it reaches RTBC status way before then)

I disagree with this because no issue that gets to "RTBC" against 8.1.x will be committable by the time 8.1.x opens. (We've discussed that 8.1.x won't even open until 8.0.2 or so; we are talking more than six months and probably at least a year. Core now won't resemble core then.) Furthermore, the fact that the 8.1.x branch even exists is solely a hack to make Drupal.org give us the option in the dropdown box. The branch isn't open for development and in a sense it hasn't even really been branched yet. It is not maintained with 8.0.x; it is months stale now:

[mandelbrot:drupal | Tue 06:35:49] $ git diff 8.0.x origin/8.1.x | wc -l
559519
[mandelbrot:drupal | Tue 06:35:59] $ git log origin/8.1.x
commit 54d5db55237679ba40995a67e27a7e48ef3a8f6b
Author: Alex Pott <alex.a.pott@googlemail.com>
Date:   Mon Jul 28 22:52:13 2014 +0100

    Issue #2310729 by tim.plunkett: Add tim.plunkett as a maintainer for Forms system, Routing system, and Configuration module.

550,000 lines and 4 months of difference means that contributors should not be creating patches against this branch. Expecting contributors to know that we will only look at 8.0.x now, and that 8.1.x isn't really open, and that even if they created patches for 8.1.x they would need to use the 8.0.x branch in git for the newer branch tip, and that their patches are going to go stale anyway... without it saying that clearly on the issue... seems like the overkill to me.

There's no need to deliberately hide the truth about these issues. They've been postponed. Our issue queue can communicate this to contributors, and it should. We've seen over and over the frustration it causes people when our issue queue suggests to them their work can get in, but it doesn't, so let's avoid that. We don't want people to waste time on issues that aren't going to land.

Edit: As per the summary:

Set guidelines to help contributors avoid focusing energy on issues which won’t be committed until 8.1.x or 9.0.x, since it can be frustrating to get a patch to RTBC only to find it won’t be committed until 6-48 months in the future.

catch’s picture

Yeah I think postponed is OK for 8.1.x and 9.x. We might open them a month or so before 8.1.x actually opens for development (no real point branching until there's an RTBC patch to commit to that branch), but this will be after 8.0.0 is actually released.

Some issues are also blocked on other patches, but that can be handled with parent/related issues and issue summaries (and we aren't great at doing that consistently with postponed anyway).

catch’s picture

Priority: Critical » Major

Leaving this open for the remaining discussion, but demoting from critical - I don't think any of these details are blocking, but having proper documentation in the first place was.

catch’s picture

Also I added a note about 'bugs and tasks that need to be backported to Drupal 7 and/or Drupal 6' to prioritized items.

I agree with xjm that any new feature for 7.x should have to follow same rules as if it was only going to be added to 8.0.0 and not backported, unless it's something that doesn't need to go into 8.x first anyway.

David_Rothstein’s picture

Priority: Major » Critical

Regarding backports, I think stuff that needs backport to D7 should fall under the "prioritized categories" already?

I think this is probably true for bug fixes and tasks (and now it's official based on @catch's change to the documentation). But not for features since the diagram says to postpone those no matter what.

Since Drupal 8 has been in feature freeze since Dec. 2012 and over feature issue thresholds for longer...

Technically, yes, this backport problem should have arisen in 2012 rather than now :) But in practice tons of features have been committed to Drupal 8 during that timeframe anyway: https://www.drupal.org/project/issues/search/drupal?status[]=7&categorie... (not all of those are actually closed due to a recent commit, and not all of those are actually features despite being labeled that way, but quite a few are)

I assume that with the Drupal 8 beta and especially with all the effort to define what can go in during the beta, means this really becomes strict now.

Essentially this is a new problem (since Drupal 6 was never really open for features post-release). Accepting new features into the current stable release but having more stringent criteria for getting things into the unstable branch doesn't make a whole lot of sense to me and also effectively blocks things from getting into the stable release. Given that new features for Drupal 7 are generally pretty small and have to be reviewed very carefully and meet stringent criteria (essentially, "don't break anything") I think it would be reasonable to still accept features into 8.0.x that meet those criteria, as long as they are backportable also; it shouldn't cause much disruption.

---

I disagree with this because no issue that gets to "RTBC" against 8.1.x will be committable by the time 8.1.x opens. (We've discussed that 8.1.x won't even open until 8.0.2 or so; we are talking more than six months and probably at least a year. Core now won't resemble core then.)

I don't think it's unusual for a patch against a stable (or almost stable) release branch to go months or even years and still apply afterwards, or at least apply with a small reroll.

The branch isn't open for development and in a sense it hasn't even really been branched yet. It is not maintained with 8.0.x; it is months stale now... Expecting contributors to know that we will only look at 8.0.x now, and that 8.1.x isn't really open, and that even if they created patches for 8.1.x they would need to use the 8.0.x branch in git for the newer branch tip,

That is a good point. I was not thinking of the 8.1.x branch that way at all, but now that I think about it, it's obvious that 8.1.x must be out of date :) Yes, I can imagine that confusing people. Perhaps it would be good to replace the whole branch with a README.txt file (and then delete and recreate the branch later when it's open for real)... Although presumably this same issue has occurred every time in the past that a new Drupal core branch was created.

I don't feel particularly strongly about not postponing 8.1.x issues, but I think it's unlikely to be applied consistently (especially with people creating new 8.1.x issues all the time) and there's potential for confusion with that also.

David_Rothstein’s picture

Priority: Critical » Major

Did not mean to change the status. I think I had this issue open in a tab in my browser for like 3 days in a row so it was very stale :)

catch’s picture

@David do you have a specific Drupal 7 feature in mind that you think ought to still be able to get in to Drupal 7, but would be blocked on 8.0.x?

David_Rothstein’s picture

Well, the two issues in the list I linked to before (there are still only two of them) both look like they would be useful for Drupal 7 if people work on them:
#299138: Improve \Drupal\Core\Utility\Mail::htmlToText()
#2212323: Make a way to better control search result display

For others that have not yet been bumped to 8.1.x (but are currently tagged as feature requests for 8.0.x so if that sticks they will be bumped), here's the full list of those that are also currently marked for D7 backport:

https://www.drupal.org/project/issues/search/drupal?project_issue_follow...

It's a pretty large list.

Skimming through it, here's a few that caught my eye that seem quite likely to be actually backportable to Drupal 7 and that wouldn't necessarily require tons of effort or be incredibly disruptive either:
#1862094: After installing a theme, display a link to "install another theme"
#1252606: Add crop anchor option to Scale and Crop image effect
#1067620: Add Description text for image fields
#205515: Usability: Link to the next new comment
#1370346: Add apostrophe as thousands marker

a_thakur’s picture

Issue summary: View changes
xjm’s picture

Status: Needs review » Fixed

We're now at the end of the beta phase. I didn't realize this issue had gotten re-opened, but we did add any issue tagged "needs backport to D7" to the prioritzed changes for the beta per David's comments.

Going forward, we should discuss if we can also adopt semantic versioning (or something similar to minor versions) for Drupal 7, in order to allow features and improvements to be added to D7 as well as D8 minor versions. I know David has some thoughts around this. That's a separate issue to discuss though.

Status: Fixed » Closed (fixed)

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

David_Rothstein’s picture

we did add any issue tagged "needs backport to D7" to the prioritzed changes for the beta per David's comments.

Hm, that's definitely good, but I think it applied to bugs/tasks more than features - the big issue is that "Feature? Yes => Postpone" was still at the top of the diagram at https://www.drupal.org/contribute/core/beta-changes.

In practice I'm not sure how much this mattered in the end since very few features actually got formally bumped to 8.1.x, but it probably had an effect. I think we should consider changing this in the future (if the diagram is still relevant then) and will create an issue for that.

Going forward, we should discuss if we can also adopt semantic versioning (or something similar to minor versions) for Drupal 7

Currently under discussion at #2135189: Proposal to manage the Drupal core release cycle although might be moved to a separate issue.

David_Rothstein’s picture