Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Comment | File | Size | Author |
---|---|---|---|
access_plugins.patch | 12.65 KB | aspilicious | |
Comment | File | Size | Author |
---|---|---|---|
access_plugins.patch | 12.65 KB | aspilicious | |
Comments
Comment #1
aspilicious CreditAttribution: aspilicious commentedComment #2
dawehnerThanks for working on that!
What is the reason to remove the defgroup?
We probably want to have () on access as well.
You know what i don't get: when to use Implements and when to use Overriddes.
Comment #3
aspilicious CreditAttribution: aspilicious commented1) This is my understanding of @defgroup/@ingroup, I can be wrong as the docs aren't 100% clear
Defgroup is used when you want to define an entire group of similar functions at once, when all the access plugins are in one file we can wrap them in a defgroup
@defgroup {
....
}
In our case the plugins are in seperate files so we just need to mark each of them being of the same group. Thats why we use @ingroup.
-------------------------------------------------------
2) When you're using @overrides, you override existing functinality, @implements is for implementing code for the first time. For example when you implement methods that are defined abstract in the parent. I see I made a mistake with "public function summaryTitle() {" as it isn't abstract method so we should use @overrides on those.
Comment #4
xjmThanks @aspilicious. Few thoughts though:
The patch is deleting the group definition, but then adding an @ingroup to that group. Doesn't make any sense. :)
I have a vague plan on how to clean up the doc groups later, but it's a post-feature-freeze task. So is renaming methods to camelCase(); we absolutely should not do that yet.
However, the method documentation cleanups in this patch are useful before then. Adding missing docblocks or parameter or return documentation are definitely worthwhile, but I'd like to keep anything else out of scope for now.
Comment #5
xjmSo from a draft post somewhere else, this is my version of the documentation gate:
I'd prefer not to have any docs or code style polish outside those things before feature freeze.
Comment #6
aspilicious CreditAttribution: aspilicious commentedCan you explain to me the difference between @defgroup and @ingroup than?
In the past we had one file:
@defgroup something {
class1
class2
class3
...
}
Now we have seperate files for each class so I thought this should be:
@ingroup something
class 1
// new file
@ingroup something
class 2
// new file
.....
Comment #7
xjm@aspilicious, sure, but an
@ingroup
that has no@defgroup
is broken. There needs to be a@defgroup
somewhere, and then the classes that belong in that group either can be inside it, or elsewhere and then added to it with@ingroup
. So actually the current format (with a defgroup wrapping a single class in a PSR-0 file) is probably fine for the main plugin class, and then classes that extend it can get@ingroup
.Alternately, we could move the
@defgroup
toviews.api.php
, and put the@ingroup
on the main plugin type class as well. There are advantages and disadvantages to that. The main disadvantage would be that we might end up duplicating documentation that should go in the class docblock for the mainplugin type class.Comment #8
xjmBut, to clarify, I'd rather not bother with implementing a change in that regard until at least feature freeze. :)
Comment #9
xjmComment #10
mgiffordPost feature freeze.
Comment #11
jhodgdonUm... What are we doing here? This issue is old. The Views plugin topics have been written, and all plugin type docs were cleaned up considerably on #2269389: [meta] Make sure plugin developer info is discoverable. Do we still need this issue?
Comment #12
mgiffordI'm just cleaning up old issues in the queue that had been left open and marked Postponed. We can mark this closed for now.