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

namespace hinterlegen #63

Merged
merged 15 commits into from
Mar 10, 2024
Merged

namespace hinterlegen #63

merged 15 commits into from
Mar 10, 2024

Conversation

alxndr-w
Copy link
Member

@alxndr-w alxndr-w commented Feb 2, 2024

To Do: Kompatibilitätsklassen hinterlegen, in nächster Major-Version entfernen.

@alxndr-w alxndr-w self-assigned this Feb 2, 2024
@christophboecker
Copy link
Member

christophboecker commented Feb 2, 2024

Hi,

hast Du den Slack-Thread verfolgt?

Demnach sollen Namespace- und Klassen-Namen mit einem Großbuchstaben beginnen und CamelCase sein.

namespace FriendsOfRedaxo\Neues;

Den Cronjob kannst Du auch in den Namespace nehmen. Funktioniert, siehe Geolocation.
https://github.com/FriendsOfREDAXO/geolocation/blob/b45f8d016374d910cc48e479f74d3bc1d37b50b9/lib/Cronjob.php#L1-L12
https://github.com/FriendsOfREDAXO/geolocation/blob/b45f8d016374d910cc48e479f74d3bc1d37b50b9/boot.php#L63-L64

Ist das Addon schon in Benutzung? Ja, nehme ich an. Dann kannst Du entscheiden, Ob Du einen harten Schnitt machen willst (sofort auf ein neuens Major-Release) oder smooth. Dann würde ich für einen Übergang vorschlagen, die alten Klassen zunächst als deprecated weiter bereitzustelen. Beispiel Focuspoint
https://github.com/FriendsOfREDAXO/focuspoint/blob/315ee0c326a829246456e4551baa77eca72a7613/lib/no_namespace/focuspoint.php#L18-L25

Dann ist der aktuelle Wunsch, die Klassennamen mit einem Großbuchstaben zu benginnen. Das ist easy, weil es PHP egal ist. Beispiel: Neuen_author.

Und Du könntest Dir noch überlegen, ddem allgemeinen Wunsch folgend den Klassennamen auf CamelCase zu ändern. NeuesAuthor. Bei Focuspoint bin ich davor noch zurückgeschreckt, könnte man aber auch durch ne Deprecated-Migrationsklasse abfedern und anmahnen.

@alxndr-w
Copy link
Member Author

alxndr-w commented Feb 2, 2024

Auf den Slack-Thread bin ich eben gestoßen und Zack, hast du schon hier geantwortet :)

@alxndr-w
Copy link
Member Author

alxndr-w commented Feb 2, 2024

Ja, hätte ich das mal 33 Tage früher gewusst. Im Prinzip kann ich mich mit jedem Feedback anfreunden, FriendsOfRedaxo\Neues und NeuesAuthor usw, wobei doch dann auch Entry:: und Author:: reichen würden, oder? Da man eh den Namespace Neues hat.

boot.php Outdated Show resolved Hide resolved
Copy link
Member

@christophboecker christophboecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Siehe Anmerkungen

boot.php Outdated Show resolved Hide resolved
install.php Outdated Show resolved Hide resolved
lib/neues.php Outdated Show resolved Hide resolved
Copy link
Member

@christophboecker christophboecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hoffentlich hat es geklappt mit den Änderungsvorschlägen. By the way habe ich noch mal nachgeschlagen, wie lt. PSR-1 die Namen geschrieben werden sollten:

  • Class names must be defined in UpperCamelCase.
  • Class constants must be defined in UPPER_SNAKE_CASE.
  • Method names must be defined in camelCase.
    Das wollte ich nur noch erwähnt haben, da Du ja eh noch über Änderungen der Klassennamne nachdenken wolltest; es greift ja noch tiefer ein. Deshalb mache ich hier keine Vorschläge.

alxndr-w and others added 11 commits March 9, 2024 13:15
Co-authored-by: christophboecker <christoph_boecker@t-online.de>
Co-authored-by: christophboecker <christoph_boecker@t-online.de>
Co-authored-by: christophboecker <christoph_boecker@t-online.de>
Co-authored-by: christophboecker <christoph_boecker@t-online.de>
Co-authored-by: christophboecker <christoph_boecker@t-online.de>
Co-authored-by: christophboecker <christoph_boecker@t-online.de>
Co-authored-by: christophboecker <christoph_boecker@t-online.de>
@alxndr-w alxndr-w merged commit 0308b2b into main Mar 10, 2024
1 check failed
@alxndr-w alxndr-w deleted the namespace branch March 10, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants