Closed adrianheine closed 10 years ago
What is this TODO list supposed to indicate? Are these things that need to be done before this is merged? Are these things you still need to do afterwards?
That's what needs to be done before this is merged.
From my point of view, the TODOs are addressed.
Looks good to me. Except that the JS is not tested at all. So I guess someone needs to verify that first
I've splitted the code up to be more testable. Where would you want those qunit tests to be put? Currently, tests/
only has phpunit tests.
Where would you want those qunit tests to be put? Currently, tests/ only has phpunit tests.
I'd move those to tests/phpunit and add a tests/qunit.
This amend approach does mean there is no diff between the old version and the new one :(
That's right (and bad), although you don't really have diffs between change revisions in gerrit, either, if you rebased the commit.
Rebased against master
Tests passing \o/
My review is still +1
Todo:
Support valueless snaks as soon as the backend does so as well