At/namespace feature - Githubissues

laminas / laminas-mvc-plugin-flashmessenger

Plugin for creating and exposing flash messages via laminas-mvc controllers

https://docs.laminas.dev/laminas-mvc-plugin-flashmessenger/

BSD 3-Clause "New" or "Revised" License

10 stars 16 forks source link

At/namespace feature #24

Closed kliker02 closed 2 years ago

kliker02 commented 2 years ago

Q	A
New Feature	yes

Description

New feature, in accordance with the issue https://github.com/laminas/laminas-mvc-plugin-flashmessenger/issues/3

froschdesign commented 2 years ago

I suggest a complete simplification and recommend adopting the changes for version 3 of laminas-view. @gsteel has done a great job and introduced good changes, which should also be implemented here:

no inheritance from the abstract helper, if not needed
set all relevant dependencies and options via constructor
removal of setter and getter methods
usage of the HtmlAttributes view helper and/or the Laminas\View\HtmlAttributesSet class

For example, see the new helper GravatarImage.

This requires a new major version.

(@gsteel, have I forgotten or overlooked something?)

kliker02 commented 2 years ago

So, shall I support backward compatibility, tests or not, @Ocramius @froschdesign?

Ocramius commented 2 years ago

IMO BC breaks should be weighed against feature impact.

If this is world-breaking, then OK, but here it's really just about supporting multiple namespaces 🤔

Let's first look at a backwards-compatible solution, before breaking everything: if it's ugly/unfeasible, we decide whether to value the feature or the stability.

froschdesign commented 2 years ago

My comment was misleading: I just wanted to point out that maybe a complete rewrite, would save another rewriting / touching.

But I agree with @Ocramius here:

Let's first look at a backwards-compatible solution…

Ocramius commented 2 years ago

Will close here: this needs a fresh start, rather than working on the existing proposal.

Ocramius commented 2 years ago

Note: please feel free to open a new PR with a new proposal.

I suggest running https://github.com/Roave/BackwardCompatibilityCheck on your patch, if you are unsure about the BC implications of your work: that helps you moving forward without being stuck.