The docblock suggests that either Iterator or IteratorArray ought to be implemented. First, there is no IteratorArray - there's ArrayIterator but (second), it's a class, not an interface, so it would have to be extended, not implemented. I've updated the docblock to reflect this, and provided a second example accordingly.

I don't actually KNOW if it's true that it'd be fine to extend ArrayIterator, but if it is, then the docblock should be spot-on.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

Status: Needs review » Needs work

Good catch: IteratorArray should be IteratorAggregate (that was a typo), but this is the only required change.

sdboyer’s picture

Yeah, given our discussion over in #341031: DatabaseStatementInterface extends Traversable for no reason, that's now quite clearly a typo :)

jhodgdon’s picture

This is quite an old issue... Is it still relevant, is the patch correct?

jhodgdon’s picture

Status: Needs work » Needs review

DatabaseStatement_docblock.patch queued for re-testing.

jhodgdon’s picture

Status: Needs review » Needs work
jhodgdon’s picture

Status: Needs work » Postponed

This needs to be postponed until standards for documenting classes and interfaces are agreed upon.
#718596: Lacking standards for documenting classes, interfaces, methods

jhodgdon’s picture

Status: Postponed » Active

Doc standards are now defined:
http://drupal.org/node/1354#classes

jhodgdon’s picture

bump.
Any thoughts on relevance or correctness of this patch -- I guess at a minimum it needs a reroll and a fix as in comment #1 above, but is it still necessary and/or a good idea?

jhodgdon’s picture

Status: Active » Needs review
FileSize
1.5 KB

Here's a new patch, which cleans up the DatabaseStatementInterface doc, and makes the change as in #1 above.

jhodgdon’s picture

#9: 341038.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 341038.patch, failed testing.

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
1.5 KB

Here's a reroll.

jhodgdon’s picture

#12: 341038-12.patch queued for re-testing.

jhodgdon’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

updating, will click retest in a sec...

jhodgdon’s picture

Issue tags: -Needs backport to D7

#12: 341038-12.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs backport to D7

The last submitted patch, 341038-12.patch, failed testing.

jhodgdon’s picture

Issue tags: +Novice

Looks like this needs a reroll. Probably a good novice project?

kathyh’s picture

Assigned: Unassigned » kathyh
kathyh’s picture

Status: Needs work » Needs review
FileSize
1.37 KB

Re-roll to clean up the DatabaseStatementInterface doc and correct to use IteratorAggregate

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

This looks OK to me, and appears to satisfy the concerns raised by Damien above, as well as the original post. Thanks!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x and 8.x. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Novice, -Needs backport to D7

Automatically closed -- issue fixed for 2 weeks with no activity.