- Issue created by @rszrama
- 🇺🇸United States rszrama
As others noted the difference in "recommended" versions on the project page itself, also bear in mind that that designation is completely unknown to Composer. Such a configuration cannot be depended on to prevent build processes from failing.
The answer, though, is simple: semantic versioning.
- 🇩🇪Germany Anybody Porta Westfalica
@AdamPS: As this reads a bit aggressive (but we all know situations as developers, running into such issues and getting angry), I'd like to clearly say THANK YOU! :)
And I'm totally sure, @rszrama absolutely respects and appreciates your great work in this module. The benefits from using semantic versioning and flaws of the current Drupal.org "supported / recommended versions" are sadly correct and we have to deal with it. So I think the suggestions make sense?
What do you think?
(Drupal.org should point that out more clearly now that Composer is used as default module installation & update method)
- 🇬🇧United Kingdom adamps
It's not entirely easy to develop a complex module like this as the sole maintainer. I'm very grateful for all those who have contributed patches, but the the main effort was mine, trying to improve and fix an area that has got a bit stuck in the past. I'm happy that now 21k sites are using the module, and mostly they seem to be working.
@rszrama I see you are frustrated, and I have empathy for your situation. Sometime in the past even I've been frustrated with maintainer decisions about Commerce😃. However I believe you are misunderstanding the situation. The two points you mentioned are entirely unrelated.
🐛 Fresh installations emails broken with 1.3.0 Fixed was a bug. It was pretty tricky to understand because it would only reproduce if you fresh installed 1.2 then upgraded to 1.3, and then it "fixed itself" on any site as soon as you tried to investigate it. So for a long time the issue was set in "postponed - needs info to reproduce". This was not a break in semantic versioning AFAIK because no one needed to change their code - they just needed to wait for the bug to be fixed.
The latest example is the removal of an entire submodule between 1.2.x and 1.3.x. This is a minor version bump that should not introduce such breaking changes.
Please explain how this is breaking? The sub-module was not removed, it was marked obsolete.
Previously there was 💬 Questions about releases and compatibility Fixed which was similar - it made claims of BC breaks, but I didn't find any concrete examples to support this. So at the moment, until someone can explain otherwise, I believe this module is respecting semantic versioning.
- 🇭🇺Hungary mxr576 Hungary
I am just trying to connect some dots... also evaluating the stability of Symfony Mailer module before we would roll it out on N+ sites.
First of all, awesome work! Thanks for trying to bring the email sending feature in Drupal core to level 2023! \o/
Please explain how this is breaking? The sub-module was not removed, it was marked obsolete.
I think what @rszrama was referring to is this unexpected side effect of marking a module as obsolete, the system does not allow installing them anymore.
https://www.drupal.org/project/commerce_kickstart/issues/3380845 →
- 🇬🇧United Kingdom adamps
I think what @rszrama was referring to is this unexpected side effect of marking a module as obsolete, the system does not allow installing them anymore.
Thanks for explaining.
Nevertheless IMHO obsolete is exactly the correct status for the sub-module: it contains no code, there's no point in installing it, however it still exists for BC - it's very much similar to marking something as deprecated. So I feel that this is a bug in module installer. It would make sense to generate a warning, but not to abort the installation. With the current code, I would have to wait until a new major version to mark a sub-module as obsolete, in which case what's the point as I might as well just remove it.
- 🇬🇧United Kingdom adamps
What's a shame about this issue is the way it's presented. The first part makes me feel like a schoolkid getting a bad report from a teacher, then there's the threat. Sure the Commerce team are capable of writing an alternative, but that would be rather divisive and I can offer a tip - it's quite a lot of work and you don't get paid😃.
This module is free and open-source. I'm working hard to make it as good as I can, but my life contains other things too. People are free to use it, or not. In return I expect to feel free from people imposing their deadlines and stresses onto me. You paid nothing for it, and nobody owes you anything😃.
I'm very open to feature requests, and constructive conversations about how to make things better. However please note that this is an ambitious project and I'm already pretty stretched. So if anyone would like thing to be better, then I encourage them to offer to help.
- It makes me happy to read comments offering thanks and appreciation
- Patches for issues are always welcome
- I would welcome a small team of "advisors" who could be available to discuss key decisions
- If anyone has a business that depends on this module, then I would welcome sponsorship. Major sponsors would get prominent credit and influence on strategy.
- It would help this project to fix these commerce issues: 📌 Allow the view display in the OrderItemTable formatter to be configured Active , 🐛 The order entity is rendered with an extra copy of the billing profile Active
- I'm still interested in feedback from the Commerce team on
CommerceOrderEmailBuilder
- 🇩🇪Germany Anybody Porta Westfalica
@AdamPS: Thank you very much for all your work and efforts here! Totally agree with #6 and #7. Don't take it personally, you're doing such a great and much appreciated job here!
I think @rszrama was in rant mode, happens from time to time to most of us. Too less sparetime, perhaps :)
Voting to close this issue and end with the constructive results from #6
- 🇦🇹Austria agoradesign
I'd like to add my 2 cents to this discussion as well, before closing the issue
A couple of days ago I've read in the Commerce slack channel about a more simple alternative to symfony_mailer, offering exactly the same basic functionality as swiftmailer. The maintainer of that module also had some pain points with this module, so decided to offer an alternative implementation. Though I personally haven't tried that and I'm not planning to do so - because for the simple reason that I haven't experienced any problems so far :)
We all know situations that you can run into problems with modules that many people use and all the other seem to not have these problems at all. Sometimes, your specific project or your general specific workflow may lead to compatibility problems with other code parts. In this situation, you may feel that module/plugin XYZ is bad and to blame to occassionally destroy your site. But that doesn't mean that most of the other users feel the same.
I must confess that I've waited quite a long time to switch from swiftmailer to symfony_mailer - I waited until I finally created or upgraded the first D10 site - because in the early days of symfony_mailer module all the new concepts just felt to heavy-weight. there wasn't a good guidance at that point, and the whole mail-sending part of a website is normally for me a very unimportant side-feature, which just has to work.
When I first used a stable version of symfony_mailer, I was surprised in a very positive way - the upgrade experience and the UI was great. Although I haven't really tried advanced use cases with specific configurations, I'm very happy about that the module can be installed easily and just does its job. It's also easy to switch the configuration in dev environments (eg provide a mailhog transport policy and enable this via local settings). It's far more mature imho than swiftmailer ever was
Finally, I want to add, that I am always relieved when I see that @AdamPS is the maintainer of a project I'll want to use. Because then I'm always very confident that the code quality is high. So thanks for your hard work on this module and any other project you maintain :-)
- 🇬🇧United Kingdom adamps
OK I tried to turn the IS into a constructive discussion. Any comments related to that are welcome.
- 🇬🇧United Kingdom adamps
about a more simple alternative to symfony_mailer, offering exactly the same basic functionality as swiftmailer.
Interesting - does anyone have a link to the project?
In some ways it's a shame as it divides the efforts of the community. However it might also be for the best in other ways because it will mean there's somewhere else for people to go - currently perhaps people feel forced to use this module because there's no alternative, but don't really like it. So it might mean that I get less hassle and grumpiness in the issue queue😃.
It will also be interesting to see what direction Core takes regarding 🌱 [META] Adopt the symfony mailer component Needs review .
- Stage 1 📌 Add symfony/mailer into core RTBC is like a simplified swiftmailer, adjusted for symfony_mailer.
- Stage 2 is a full new mail system like this module. I invested some time describing how Core could adopt some parts of this module with modifications. However I realise that it's probably a difficult thing to get committed.
- 🇭🇺Hungary mxr576 Hungary
about a more simple alternative to symfony_mailer, offering exactly the same basic functionality as swiftmailer.
Interesting - does anyone have a link to the project?
from my todo list =] https://www.drupal.org/project/symfony_mailer_lite →
- 🇦🇹Austria agoradesign
here's the link: https://www.drupal.org/project/symfony_mailer_lite →
regarding Core discussion: it's a shame imho that we need a contrib module anyway. HTML e-mails and configuring the mail transport (SMTP or whatever) must be part of core normally. Would be great to see stage 1 coming asap, leaving enough flexibilty for contrib modules like this one to add extra features like defining policies
- 🇬🇧United Kingdom adamps
Thanks for the link.
it's a shame imho that we need a contrib module
Yes, people are trying to fix it, but Core moves very slowly😃.
- With stage 1, Contrib can add configuration of the mailer transport
- With stage 2, Contrib can add configuration of Mailer policy - stage 1 doesn't provide the right APIs.
- 🇺🇸United States rszrama
Sorry my original post wasn't clear and caused undue offense. I honestly was surprised to see @Anybody regard it as an emotional outburst. I wasn't angry when I wrote it and wouldn't consider it a rant. To clarify: my goal was to explain the reality facing me as a distribution maintainer: a module not following semantic versioning in the same way as every other project in the Drupal ecosystem is a hinderance to maintainability. Our only recourse would be not to use such a module, whether it's Symfony Mailer or any other project.
Re: the marking of a module as obsolete, as mxr576 mentioned, that did indeed start breaking Commerce Kickstart installations. I see that @AdamPS thinks this was the right approach, but I don't know what that assumption is based on. The documentation for the lifecycle feature → describes "Deprecated" modules as those that will be removed from a future major release and "Obsolete" as those that cannot be installed any more but "are only kept for backwards compatibility for existing sites." For a site already running this project, there's little risk, though removing all of the module's code doesn't satisfy "backwards compatibility" - but what about projects, whether distributions or other contributed modules, that have a dependency on a module that they expect to be installable based on a semantic version constraint?
The pattern in Drupal core (and by extension, the contributed module ecosystem, including Commerce) is to first mark a feature or module as deprecated and then to remove it in a future major release, as the documentation notes for deprecations (emphasis mine):
Deprecated extensions are those that are currently provided in Drupal, but will be removed in a future version. These extensions can still be installed on a website, but it is important to note that they are likely to be removed in a future major release.
(And honestly, it's the lifecycle feature in Drupal that's a bit odd here, similar to the issue with the "recommended version" specification for releases... there's likely a core or d.o documentation issue we could open out of this.)
However, Semantic Versioning is clear that minor versions such as a bump from
1.2.2 -> 1.3.0
are for "when you add functionality in a backward compatible manner". If a module could be installed as a dependency of another project depending on^1.2
(i.e. any patch or minor version at or above1.2
that is less than a2.0
), then it breaks backwards compatibility to make that module uninstallable in a new minor version.Now, I said "little risk" above, but it's also worth pointing out that it was part of the module's public API that it defined a new service,
symfony_mailer_bc.overrider
. Imagine a scenario where a developer decorated that service with their own class, extending yours to add some additional functionality. Given the nature of that particular submodule, it's unlikely, but it is the kind of thing that is permissible in Drupal and protected by semantic versioning. If that site depended on^1.2
and then the class were removed from the site's codebase with the1.3.0
update, the custom code would break. Hopefully that would be caught during a build process prior to deployment, but removing the service / class even if a.info.yml
file remained was by definition a "non-backward compatible" change.I'm not trying to rant here despite the length of the comment. I just don't know how to explain what seems obvious to me without using so many words - making a module uninstallable and removing all of its code is not backwards compatible and therefore should warrant a major version bump.
I'll leave it to y'all to figure out the path forward, but I'll reiterate my initial conclusion that was meant as an encouragement to use as many version numbers as needed: there's no harm in creating any number of new major versions, but there is harm in not respecting semantic versioning's expectations for backwards compatibility between minor releases.
- 🇬🇧United Kingdom adamps
Thanks @rszrama for a friendly clarification. I can absolutely confirm what @Anybody said in #3 - I did find your manner of raising this issue to be aggressive and upsetting. What's more, please bear in mind that it's definitely not the best way of persuading people to do what you'd like😃. If you'd paid for a product then that would be different.
1) Thanks for pointing out the documentation of the lifecycle field which I was previously unaware of. I now see that marking symfony_mailer_bc as obsolete was my mistake, apologies, and I should have used deprecated. There are still 10k sites on 1.2 so this is probably worth fixing seeing seeing as it's easy. I have raised 🐛 symfony_mailer_bc should be deprecated not obsolete Active .
When I said that I believed obsolete was correct, I meant that from the point of view of the natural, intuitive meaning of the words in the English language that obsolete is the correct description of the status of the module. However the documentation page you refer to obviously takes precedence over my intuition😃.
2)
Imagine a scenario where a developer decorated that service with their own class, extending yours to add some additional functionality or make use of it in another context.
Absolutely, you are right from an theoretical point of view. And in 📌 Drupal 10.1.0 new aggregation breaks InlineCssEmailAdjuster Needs review @TwoD was right to point out that theoretically every time we want to write 1 line of code that uses a new service, in addition to the 15 lines of boilerplate code to inject it, there should be another 5 lines of boilerplate for deprecation, which then need to be removed in the next major version. On the other hand, @Niklan who was giving his time to write the patch, clearly didn't agree. Anyway, both of these cases are fairly deep customisation of this module that is likely quite rare and probably it implies a site that brings income and retains developers.
I just don't know how to explain what seems obvious to me
It's obvious to you from your shoes - you run a business, you have a team of developers, you get paid to maintain a Drupal module. Also please note that although you said that version numbers are cheap, Commerce is still on only the second major version since D8 and has dropped support for the first one. I believe that in reality that maintaining multiple versions has a non-trivial cost, and so does writing BC code. The same for release notes and change records (yes I wrote them, although perhaps many people don't read them then complain when things go wrong😃).
Please try seeing it from my shoes😃. This is a project run in my spare time which brings me no income. It already takes more time than I would prefer so intrudes in the rest of my life, yet I have a passion to try and make it work, to find a balance. So there may be a necessity to reduce the maintenance/developers cost and move some burden to the sites making deep customisations. I have raised 📌 Consider marking some classes/functions @internal and find more convenient ways of dependency injection Active . Even if we do this, I believe this module is vastly more configurable and customisable than any other alternative. For example, until the recent fixing of ✨ Provide a way to customize the order receipt email subject Fixed , this module was the only way to change the commerce order receipt email subject using the admin UI.
3) In your role as manager of Drupal Commerce, please recommend whatever email module you think will best serve your users.
I suggest that a lot of the "pain" felt by sites using this module was caused by your past decisions. Based on your earlier issue, apparently you were recommending this module at a time when it was an early alpha and clearly documented as unstable - in your distribution which I assume is a stable release. This would also be a mistake in terms of semantic versioning and back-compatibility.
In the past, this module has been changing fast yet with a large installed-base due to EOL of swiftmailer. The good news is that in the future things should be much more stable as I'm not planning any more big changes.
- 🇬🇧United Kingdom adamps
As I said in #7, if people wish to improve this project, please don't complain instead help.
Specifically in terms of BC
If you have a problem with BC, please raise an issue ASAP with a constructive detailed description. (Instead of raising months afterwards without full details.) The problem can often be corrected if it is known. Or I might explain how I don't think it's a BC break. I remind people once again that in 📌 Drupal 10.1.0 new aggregation breaks InlineCssEmailAdjuster Needs review , a Drupal Core minor release totally broke this module and any other that sends email with CSS. Yet @catch was insistent that this didn't break the Core BC policy.
Ways to help that would be appreciated
Repeated from #7:
- It makes me happy to read comments offering thanks and appreciation
- Patches for issues are always welcome
- I would welcome a small team of "advisors" who could be available to discuss key decisions
- If anyone has a business that depends on this module, then I would welcome sponsorship. Major sponsors would get prominent credit and influence on strategy.
- It would help this project to fix these commerce issues: #3363490: Allow the view display in the OrderItemTable formatter to be configured, #2949726: The order entity is rendered with an extra copy of the billing profile
- I'm still interested in feedback from the Commerce team on CommerceOrderEmailBuilder
- 🇬🇧United Kingdom adamps
My last posts were long so I'll try for a brief summary.
- I think we all agree that semantic versioning is important.
- 🐛 symfony_mailer_bc should be deprecated not obsolete Active was my mistake. All it needed was for someone to raise the issue explaining that "obsolete" didn't do what I expected.
- It's important to decide what semantic versioning applies to. I would say absolutely yes to all interfaces. I didn't necessarily expect it to apply in all other cases as also developers need scope to change things.
- This module is big for me alone (and also there's 🌱 [META] Adopt the symfony mailer component Needs review ). I would welcome co-maintainers, or at least a forum where I can discuss strategy and ask for review of designs.
Let's keep this issue open for any future debate. I can ask for suggestions for a particular issue and other people can raise changes they feel were non BC.
- 🇺🇸United States rszrama
If you'd paid for a product then that would be different.
Wait, you mean people use Drupal modules without paying for them? 🤯
- 🇺🇸United States rszrama
I'll see if I can find a channel to get the
lifecycle
feature documentation improved as well. Was just discussing expectations for backwards compatibility across major Drupal versions earlier today in Slack, too. - 🇬🇧United Kingdom adamps
Of course I'm not expecting to be paid by everyone who uses this module. I'm just wondering if, seeing as people are getting something for nothing, they might reduce the threats, demands and complaints😃. I'm happy to receive constructive feedback and offers for help.
I expect most of the people on this thread maintain modules, so you know the deal. Some people press for a stable release sooner, some then complain that it's not stable enough, some people want maximum stability and compatibility (with swiftmailer in this case), some people want maximum features ASAP, some people want all of these things. People are quick to raise an issue the minute their site is broken after upgrade, complaining it's non-BC without taking the trouble to work out why it broke - maybe it's simply a bug, maybe they used an alpha, maybe they didn't read the release note, maybe they aren't using the module in the way it's intended.
It's true that in a minor release I chose to alter some "internal-ish" classes in a way that could impact sites, but hopefully just a handful or even none. I believe this was documented in the release note. This was not because I'm totally ignorant of the implications, rather it was trade-off. It helped me to make important new features available sooner without waiting for a major release and saved me time to get more done. To be honest I have forgotten the details, but even with hindsight it doesn't seem an absurd choice. Anyway I'm the one putting in the work, so for better or worse I get to make the decisions and I don't feel obliged to justify each one.
This module is quite an ambitious project and it became popular quite quickly starting even when it was in alpha. Mostly I have no one to consult to check my ideas and make the key decisions alone - so any poor choices, mistakes or gaps in knowledge are not caught. I apologise to everyone for the pain caused, but still, overall, I feel pleased. I have fulfilled my initial plans, and sites are finding the module useful.
I'm doing my best to keep everyone happy, and the good news is that in future things will change less and should be more stable. Still maybe this module won't be for everyone. For example maybe some people want something as similar as possible to swiftmailer meaning Drupal Symfony Mailer Lite, and I already opened a friendly discussion with the maintainer.
I a fellow human being trying to have fun and get fulfilment from offering something I'm passionate about to the world. Whether you end up using this module or not, let's be nice, let's be friends😃😃
Anyway I'll try to make that my last rambling comment and leave this issue for its intended purpose.