Via issue comments and our contact form, @john.oltman and I have considered the possibility of an 8.x-3.x branch for a bunch of my recent patches. Starting a public issue to track that discussion.

I'm personally fine adding features to a "stable" release, and even breaking interface translations. Many contribs do. But, it's probably nicer to leave 8.x-2.x stable and start a new 8.x-3.x. It might be trouble maintaining both branches for any serious length of time. If we're going to deprecate 8.x-2.x and suggest people upgrade to the 3 series for ongoing support, it's not much different than adding features and even breaking strings inside the 2.x series. :/ Ultimately, it's up to john.oltman. ;)

If we do create an 8.x-3.x branch, here's a draft proposal of what currently open issues should go where:

8.x-2.x

#2969594: Simplify entity rendering code in ForwardForm::submitForm()
#2972096: Fix config export to only show entities where forward is enabled

8.x-3.x

#2969352: Add option to allow plain text emails (I'd move the initial commit to the 3.x branch, remove it from 2.x, and commit the follow-up fix only to 3.x).
#2969418: Allow a separate filter format for plain text vs. HTML emails
#2969419: Consistently enforce the selected filter format by using it during entity rendering
#2971226: Add a setting for an email footer textarea
#2971228: Provide #recipient as a variable for the forward twig template (maybe this is harmless enough to be backported?)
#2971847: Add forward template suggestions for HTML vs. plain

Comments

dww created an issue. See original summary.

dww’s picture

dww
we/he/they
commented
Issue summary: View changes

Note about moving the initial commit for #2969352: Add option to allow plain text emails out of 8.x-2.x branch into 8.x-3.x if we go forward with this.

john.oltman’s picture

john.oltman commented

I am definitely in favor of possibly doing one more 2.x release (with no translation breakage) and then everything else moves to 3.x. I suppose I may beed to revert some commits on 2.x if we go this route, which is no problem. I very much appreciate all the thought you are putting into this. I have been swamped but should be able to tackle this soon (within the next few days) unless we hear objections.

dww’s picture

dww
we/he/they
commented
Issue summary: View changes

Sounds good.

Added #2972096: Fix config export to only show entities where forward is enabled as something else for 8.x-2.x, since it's not breaking any strings.

I think the only commit that's already in 8.x.2.x that shouldn't be is 76fb5bc79d23c4b4. The others you've already committed aren't string-breaking, and are either bug fixes or very minor new features.

Most of the rest of my patches should probably go into 8.x-3.x branch and be released as 8.x-3.0-alpha1 or whatever. ;)

Thanks!
-Derek

john.oltman’s picture

john.oltman commented

Great. I will get a new 2.x Dev release and a 3.x alpha release going soon. Thank you.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Reviewed & tested by the community

If it helps, here's the order of patches I'm applying on the main site where I'm using all this, care of the patches section of my composer.json:

      "drupal/forward": {
        "#2968459-2: Add tokens for links to the entity you're forwarding Add tokens for links to the entity being forwarded" : "https://www.drupal.org/files/issues/2018-05-01/2968459-2.forward-link-tokens.patch",
        "#2968659-2: The 'link' variable in forward.twig.html should be absolute The 'link' variable in forward.twig.html should be absolute" : "https://www.drupal.org/files/issues/2018-05-01/2968659-2.forward-entity-link-absolute.patch",
        "#2969005-2: Migrate source plugins need to define a 'source_module' in their annotation Migrate source plugins need to define a 'source_module' in their annotation" : "https://www.drupal.org/files/issues/2018-05-02/2969005-2.forward-migrate-source-module.patch",
        "#2969317-2: Convert forward to use short array syntax Convert forward to use short array syntax" : "https://www.drupal.org/files/issues/2018-05-03/2969317-2.forward-short-array.patch",
        "#2969337-6: 'Allowed HTML tags' should only appear if both 'Personal message' and 'Allow HTML in personal messages' settings agree Settings form UI improvement for dependent config options" : "https://www.drupal.org/files/issues/2018-05-03/2969337-6.patch",
        "#2969352-4: Add option to allow plain text emails Add option to allow plain text emails" : "https://www.drupal.org/files/issues/2018-05-03/2969352-3.allow-plain-text.patch",
        "#2969352-10: Add option to allow plain text emails Follow-up fix for plain text emails" : "https://www.drupal.org/files/issues/2018-05-04/2969352-10.plain-text-needs-helper-class.patch",
        "#2969594-2: Simplify entity rendering code in ForwardForm::submitForm() Simplify entity rendering code in ForwardForm::submitForm()" : "https://www.drupal.org/files/issues/2018-05-04/2969594-2.simplify-entity-rendering.patch",
        "#2969418-2: Allow a separate filter format for plain text vs. HTML emails Allow a separate filter format for plain text vs. HTML emails" : "https://www.drupal.org/files/issues/2018-05-03/2969418-2.filter_format_plain_text.patch",
        "#2971226-3: Add a setting for an email footer textarea Add a setting for an email footer textarea" : "https://www.drupal.org/files/issues/2018-05-07/2971226-3.email-footer-setting.patch",
        "#2971228-3: Provide #recipient as a variable for the forward twig template Provide #recipient as a variable for the forward twig template" : "https://www.drupal.org/files/issues/2018-05-07/2971228-3.forward-recipient-template-variable.patch",
        "#2972096-2: Fix config export to only show entities where forward is enabled Fix config export to only show entities where forward is enabled": "https://www.drupal.org/files/issues/2018-05-10/2972096-2.forward-clean-settings.patch"
      },

That should be the right order to apply all the patches. ;)

Cheers/thanks,
-Derek

john.oltman’s picture

john.oltman commented
Version: 8.x-2.x-dev » 8.x-3.x-dev
Status: Reviewed & tested by the community » Fixed

Created the 3.x branch. Please pull that down and confirm it matches the source code you have now when patched manually via composer. The patch ordering you provided was invaluable. Agree on the one commit to revert on 2.x which I already did. I want to work with 2.x a bit more before adding more patches to it, although I agree that the config export patch is a good candidate since it doesn't break translation. I will add that patch once I test it. Thanks again!

dww’s picture

dww
we/he/they
commented

Both branches look great, thanks!

Confirmed that the end of the 8.x-3.x branch is the same as my locally patched version with composer. Great! Glad that the patch ordering helped. ;)

I moved a few issues back to 8.x-2.x that I think are worthy of a backport. More reviews/testing would be welcome on those.

The only things I'd like to fix before an 8.x-3.0-X official release of some kind would be:
#2969419: Consistently enforce the selected filter format by using it during entity rendering
#2972504: E-mail header for HTML still set, even for plain text emails

Should we turn this into the "8.x-3.0-alpha1" planning issue, start a new one for that, or not worry about it?

Thanks!
-Derek

Status: Fixed » Closed (fixed)

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

john.oltman’s picture

john.oltman commented
Assigned: Unassigned » john.oltman

Rethinking this, your earlier comment "It might be trouble maintaining both branches for any serious length of time" was prescient. In hindsight I think it was fine to stage some ideas and code in 3.x but ultimately it isn't viable long term. Since you are the only one using 8.x-3.x, I would like to merge everything unique to 3.x back into the 2.x branch and make it the 8.x-2.3 release. For the issue with the recipients in the template when there are multiple, I think it's fine for the multiple case to simply have them be comma-delimited. The rest of it I think could be merged with few issues. If you don't disagree @dww with my plan, I'll create a new issue for the merge. I will do the work but request your help with the testing of 2.3 release candidate (if you don't mind yet more testing - sorry about this). I am guessing it will be a couple weeks before 2.3 rc is ready.