Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation iterator #1935

Closed
staabm opened this issue Jul 29, 2018 · 13 comments
Closed

Navigation iterator #1935

staabm opened this issue Jul 29, 2018 · 13 comments

Comments

@staabm
Copy link
Member

staabm commented Jul 29, 2018

Skerbis hat geäußert dass er die klasse gerne verwendet und sich diese im core wünscht

https://gist.github.com/staabm/774655974d7af5728953f6a3e3accca4

@skerbis
Copy link
Contributor

skerbis commented Jul 29, 2018

Ihr hattet auch darüber bereits diskutiert irgendwo. Ich finde sie ist der aktuellen Navigation factory überlegen und bietet viel mehr Spielraum und ist mehr redaxoiger :-)

@skerbis
Copy link
Contributor

skerbis commented Jul 29, 2018

ah: #1352 hier war's

@tbaddade
Copy link
Member

Ich wäre auch dafür, aber es schließt sich aktuell selbst aus.
Core Min PHP 5.5.9
Iterator Class Min PHP 7

@olien
Copy link
Contributor

olien commented Jul 30, 2018

Bin auch dafür!

@staabm
Copy link
Member Author

staabm commented Jul 30, 2018

Ich wäre auch dafür, aber es schließt sich aktuell selbst aus.
Core Min PHP 5.5.9
Iterator Class Min PHP 7

man kann/könnte den code des iterators noch für 5.5+ anpassen. bin allerdings gar nicht so sicher ob man das 100% brauch.

Da der core selbst die klasse überhaupt nicht verwendet, könnte man auch meiner Meinung nach sagen, dass die klasse php7 erfordert.

@tbaddade
Copy link
Member

tbaddade commented Jul 30, 2018

Wenn der Core die mind. Version mit 5.5.9 angibt, sollte m.E. auch alles was der Core mitbringt mit 5.5.9 laufen.

Oder aber wir erhöhen die Core min Version auf 7. Oder, wir machen ähnlich wie bei der 4er damals, ein AddOn "Core-PHP-7" wo eben solche Klassen mit enthalten sind.

@alxndr-w
Copy link
Contributor

http://php.net/supported-versions.php

image

Core Version auf 7.0/7.1 erhöhen erscheint angemessen. Sowohl 5.6, als auch 7.0, erhalten ab 2019 keine Security Fix mehr.

@staabm
Copy link
Member Author

staabm commented Jul 30, 2018

nur wegen dieser klasse würde ich die min version nicht anheben.
dann lieber die klasse kompatibel machen, wenn wirklich die Meinung vorherscht dass man so eine klasse nicht php7+ haben darf.

ausserdem sollten wir bei min-php versionserhöhung eine neue major version verwenden, d.h. REDAXO6. das würde ich solange hinauszögern wie möglich bzw. solange bis wir wichtige gründe finden die min-version zu erhöhen.

@tbaddade
Copy link
Member

@alxndr-w
Copy link
Contributor

@staabm gibt es vielleicht noch andere Gründe, weshalb eine Major-Version sinnvoll wäre? Ansonsten wäre ein Ergänzungsaddon eine zumindest praktikable Lösung.

@gharlan
Copy link
Member

gharlan commented Jul 31, 2018

Ich wundere mich etwas, dass die Rückmeldungen so positiv sind, da die Beispiele invalides HTML liefern (siehe Kommentar von @tbaddade unter dem gist). Ich dachte, einige hätten die auch schon richtig im Einsatz?
Ich sehe auch nicht, wie man mit der Klasse aktuell das gewünschte Markup hinbekommen könnte. Eventuell übersehe ich aber auch was.

@tbaddade
Copy link
Member

@staabm Wäre es sinnvoll hier einen PR WIP aufzumachen, so dass wir die class verfeinern und weitere Ideen einfließen lassen können?

@staabm
Copy link
Member Author

staabm commented Aug 3, 2018

#1941

@staabm staabm closed this as completed Aug 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants