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.
Per http://drupal.org/node/1112854#comment-4882080 we need to add doxygen to every function in select.inc. This is trivial most of the time, just copy-paste, there's say
/* Implementations of QueryConditionInterface for the WHERE clause. */
now remove this and instead
<?php
/**
* Implements QueryConditionInterface::conditions() for the WHERE clause.
*/
public function &conditions() {
return $this->query->conditions();
}
*/
like that.
Comment | File | Size | Author |
---|---|---|---|
#14 | select-doxygen-1255524-14.patch | 21.57 KB | xjm |
#12 | 125524-select.inc-doxygen-012.patch | 21.55 KB | posulliv |
#3 | 1255524-select.inc-doxygen-003.patch | 11.01 KB | adamdicarlo |
#2 | select.inc-needs-more-doxygen-1255524.002.patch | 5 KB | karschsp |
Comments
Comment #1
karschsp CreditAttribution: karschsp commentedComment #2
karschsp CreditAttribution: karschsp commentedPatch attached.
Comment #3
adamdicarlo CreditAttribution: adamdicarlo commentedI don't think the first patch is exactly what's needed here.
Here's how I think it should go. I've only rolled the new comments for the SelectQueryExtender class, so this new patch is not done yet. I had to look up the implemented classes to see which class exactly was the origin of the method that's being implemented. I think I did it OK.
@chx is this pretty much what's needed?
If so, @karschsp, want to roll the next off of this? The SelectQuery class still needs to be done.
Comment #4
adamdicarlo CreditAttribution: adamdicarlo commentedActually, this does need review before we should proceed.
Comment #5
catchDo we really need this? In the class definition it specifies which interface it implements (or extends from another class that specifies this) that's enough to go on IMO.
Comment #6
adamdicarlo CreditAttribution: adamdicarlo commented@catch I had assumed the idea was to make the api.drupal.org docs easier.
But looking at http://api.drupal.org/api/drupal/includes--database--select.inc/class/Se..., it looks like the api site is automatically showing what each method is overriding...
So what could we gain from adding these comments? (Anything?)
Comment #7
chx CreditAttribution: chx commentedWhen I look at api.drupal.org I see a lot of empty lines this patch would fill in .
Comment #8
catchI feel like api module should be able to figure out something from the class hierarchy (ideally a link to the respective method in QueryAlterableInterface).
What is more of a concern to me is there's no way to browse to QueryAlterableInterface on api.drupal.org
http://api.drupal.org/api/drupal/includes--database--select.inc/interfac...
vs.
Comment #9
chx CreditAttribution: chx commentedwell, certainly it'd be awesome if api module could figure out this but right now this is a very easy cop out which we can clean up once that happens.
Comment #10
catchThat's fine with me, I'm not keen on the existing:
so better to replace them with something standard too.
Comment #11
chx CreditAttribution: chx commented@adamdicarlo @karschsp please continue. we have agreement this is what we need.
Comment #12
posulliv CreditAttribution: posulliv commentedMade an attempt at this. Does the attached patch look ok?
Comment #13
xjmCrossposting #1316902: Clean up API docs for lib/Drupal/core/Database. Also, this is more a docs issue.
Comment #14
xjmAnd, a core/ reroll for convenience.
Comment #15
jhodgdonThe idea of this patch is correct. See
http://drupal.org/node/1354#classes
for standards.
But I don't know if the patch is correct. This is documentation in SelectQueryExtender, which is declared as:
So how/why are methods here saying they are implementing methods from other interfaces besides SelectQueryInterface? Such as:
That can't really be right. If this class is implementing QueryAlterableInterface, then it should declare that. If not, the documentation is wrong.
Comment #16
chris.leversuch CreditAttribution: chris.leversuch commentedIf I've understood this correctly, SelectQueryExtender implements SelectQueryInterface which extends QueryAlterableInterface. QueryAlterableInterface is where hasAnyTag() is originally defined.
The current comments are just skipping SelectQueryInterface because it doesn't include its own implementations of the functions - technically SelectQueryExtender::hasAnyTag() is overriding QueryAlterableInterface::hasAnyTag(). I'd probably comment as 'Overrides SelectQueryInterface::hasAnyTag()' even though this doesn't actually exist because it could in the future - not sure if this would confuse api.d.o though.
Comment #17
jhodgdonActually, at this point this should be included in the big Docs cleanup instead of being its own issue.
#1317628: Clean up API docs for includes directory, files starting with N-Z
Comment #18
jhodgdonOh wait, this is part of the database one, not the includes N-Z:
#1316902: Clean up API docs for lib/Drupal/core/Database