-
Notifications
You must be signed in to change notification settings - Fork 3
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 classes #10
Conversation
Sieht für mich gut aus. Hab anstatt Kommentare mal versucht auf dem aufzubauen (siehe letzten Commits). Git diff hat etwas Mühe, das Ganze sauber darzustellen.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gefällt mir sehr gut! Merci! Ist tatsächlich viel simpler so wenn wir die Dependencies explizit übergeben, das passiert gar nicht an so vielen Orten wie ich dachte. Und so ists viel sauberer weil wir später für #2 die einzelnen Klassen einfach fix exportieren können.
Mir haben noch die $reload
, $post
etc. Methoden auf den LoadingStoreValues gefehlt, sowie Tests dafür. Diese habe ich jetzt ergänzt. Der PR ist aus meiner Sicht ready zum mergen.
LGTM |
WIP
Open this as a pull request do have a place for discussion