In my opinion according to the best drupal practices the better way to realize dependency injection to PollRecentBlock plugin.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | poll_recentblock_di_2944695_11.patch | 4.7 KB | nkoporec |
| #11 | poll_interdiff_8-11.txt | 1005 bytes | nkoporec |
| #8 | poll-PollRecentBlock-dependency-injection-2944695-8.patch | 3.1 KB | mylies |
| #5 | poll-PollRecentBlock-dependency-injection-2944695-2.patch | 2.54 KB | mylies |
Comments
Comment #2
mylies commentedAnd a patch for this)
please, have a look
Comment #4
berdirLook like the wrong patch?
You don't need to add multiple test runs, only when that really makes sense, like it is something that is different for sqlite/postgresql or different drupal core versions.
Comment #5
mylies commentedeps, sorry - I miss the project folder
and a correct patch
Comment #6
mylies commentedComment #7
berdirThe create and construct method should be above all other methods and usually __construct() is first.
those need descriptions, start with what the parent has.
Comment #8
mylies commentedI changed a patch according to #7 comment
Comment #9
nkoporecTested the latest patch and it applies cleanly.
Comment #10
berdirDo not use private properties, use protected.
The docblock of the property is not following coding standards.
Also, the description on the construct method is too much. Everyone knows how constructor injection works, just say "Construct a new X object" or something like that.
Comment #11
nkoporecUpdated the patch from @myLies and fixed the issues that @Berdir has found.
Comment #13
berdirCommitted.