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.

Files: 
CommentFileSizeAuthor
#19 DBStatement_docblock_341038-19.patch1.37 KBkathyh
PASSED: [[SimpleTest]]: [MySQL] 33,308 pass(es).
[ View ]
#12 341038-12.patch1.5 KBjhodgdon
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 341038-12.patch. See the log in the details link for more information.
[ View ]
#9 341038.patch1.5 KBjhodgdon
FAILED: [[SimpleTest]]: [MySQL] 26,479 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
DatabaseStatement_docblock.patch1.27 KBsdboyer
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch DatabaseStatement_docblock.patch.
[ View ]

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
StatusFileSize
new1.5 KB
FAILED: [[SimpleTest]]: [MySQL] 26,479 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

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
StatusFileSize
new1.5 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 341038-12.patch. See the log in the details link for more information.
[ View ]

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
StatusFileSize
new1.37 KB
PASSED: [[SimpleTest]]: [MySQL] 33,308 pass(es).
[ View ]

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.