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

Refactor to make use of OOP and modern best practices #17

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tfrommen
Copy link
Collaborator

@tfrommen tfrommen commented Jun 5, 2018

Huhu! 👋

Wie bereits angedroht abgesprochen, hier der erste Commit bzgl. des Refactorings.

Es gibt noch eine ganze Reihe Sachen zu besprechen und zu machen, aber funktional ist das Plugin bereits jetzt schon vollständig. Ich habe nebenbei auch #10 umgesetzt (weil das eben ein gutes Beispiel für die Vorteile von OOP war).

Was ich noch auf der Liste habe:

  • Kopfbereich der Hauptdatei aufräumen.
  • Deutsche Übersetzung (für languages/)?
  • Travis CI?
  • Unit-Tests? (Das wäre definitiv etwas für einen separaten PR, nachdem/falls dieser hier gemerget wurde.)
  • Richtige Daten in die composer.json eintragen.

Wenn es Fragen gibt, immer her damit. Sollte der PR irgendwann akzeptiert werden, werde ich ja den erwähnten Blogbeitrag über die gesamte Umstrukturierung, meine Entscheidungen, die Vorteile etc. schreiben.

Bin auf eure Meinungen gespannt! Entweder hier oder via Slack. 🙂

@2ndkauboy
Copy link
Contributor

Why have you included the vendor folder? Shouldn't that be part of the build process?

@tfrommen
Copy link
Collaborator Author

tfrommen commented Jun 5, 2018

@2ndkauboy vendor contains only the autoloader. I did include it so that the repo can just be taken as is, and the plugin can be used without any further step.

If this repository should only contain development code (which I am a huge fan of, personally) then yes, it should not be included at all. That said, currently, there is no build process, and I didn't want to make up one in the course of this PR. 🙂

@Zodiac1978
Copy link
Member

Ich habe die Geschichte hinter diesem Refactoring nicht mitbekommen. Wie sieht es hier aus? Macht es Sinn, das nochmal aufzunehmen, @tfrommen? Würde gerne ein paar Probleme fixen und das Plugin wieder funktionsfähig bekommen.

@tfrommen
Copy link
Collaborator Author

tfrommen commented Oct 24, 2022

@Zodiac1978 keine Ahnung. Der Code steht zur Verfügung. Ich habe aktuell leider keine Zeit, mich hier weiter drum zu kümmern, sorry.

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.

3 participants