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.
similar to http://drupal.org/node/314244 etc...
the tests running not perfect
Undefined property: stdClass::$signature
no documentation changed etc.
Comment | File | Size | Author |
---|---|---|---|
#16 | trigger_broken_fix.patch | 1.15 KB | dawehner |
#15 | drupal._trigger_comment.patch | 1.15 KB | sun |
#10 | remove_hook_comment_op.patch | 9.65 KB | Anonymous (not verified) |
#7 | remove_hook_comment_op_7.patch | 9.63 KB | dawehner |
#6 | remove_hook_comment_op_6.patch | 5.48 KB | dawehner |
Comments
Comment #1
dawehnerthis code cannot work :(
wonders why not test fails
next version, not expections here
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentednice work, all tests pass.
is it possible to use a better name for
$a1
?the doc block needs updating. trigger_comment is now a helper function. might even be worth making it
_trigger_comment
.Comment #3
dawehnerthx for the review
i named all arguments for validate update insert to form_values and all other $ops to $comment
i renamed the trigger_comment to _trigger_comment and added some comments and removed the Impl. of hook_comment()
Comment #4
dawehnersry for missing the patch
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedok, nearly there.
we know the passed in
$form_values
is an array, we don't need theis_array
checks.nitpicking, but we need a blank line in the docblock between "Helper function ..." and the @param part. also, two spaces before "Argument, which will be passed to the action.". more importantly,
$a1
can only be one of two things, so best to describe that - form values or comment object.otherwise, looks good to go.
Comment #6
dawehnerthx for review again!
i changed also the the text
Reindex the node when comments are added or changed.<code> to a more specific comment for example: <code>Reindex the node when comments are deleted.
working at comment.api.php right at the moment
Comment #7
dawehner- rewrite the documentation on comment.api.php
- added some more examples
and the stuff at #6
Comment #8
dawehnerComment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedi get an exception when running search tests:
seems the doco here http://api.drupal.org/api/function/hook_comment/7 is wrong: $op publish passes an array, not the comment object.
otherwise, all looks good.
Comment #10
Anonymous (not verified) CreditAttribution: Anonymous commentedok, seems lazy not to fix that. patch attached, all tests pass.
i think this is RTBC.
Comment #11
dawehnerthx
i also think this is RTBC
Comment #12
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!
Comment #13
Dries CreditAttribution: Dries commentedComment #14
sunAll tests pass? Seems we need some more tests then. HEAD is broken.
Comment #15
sunFixed for me.
Comment #16
dawehnermh
:(
here is a patch
Comment #17
dawehnerComment #18
Anonymous (not verified) CreditAttribution: Anonymous commentedouch! we're definitely in need of more tests.
@dereine: are you ok to try to write some tests that fail for the broken code?
Comment #19
dawehnerit would be possible to write the nearly the same tests which are in trigger.test for comment actions
i do some at the moment
wonders whether it is possible to create a commit hook in cvs which checks whether each function exists and stuff like this
Comment #20
sunFirst of all, HEAD should be fixed, as this typo prevents tests for #8: Let users cancel their accounts from passing.
Comment #21
webchickCommitted to un-break broken HEAD. Leaving CNW so we can get some tests.
Comment #22
sun.
Comment #23
andypostTests are incomplete till #601398: Simpletest does not allow assigning actions to triggers
Comment #24
marcvangend#301398: Install error: Call to undefined function content_notify(). has been fixed.
Comment #25
catchComment #26
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedComment #27
catchMarking fixed, since there is a more up-to-date issue that deals with the actions breakage in comment module, and it needs more than tests sadly. #244093: Node and comment actions are (still) completely broken and have broken tests too.