Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
similar to #314244: remove $op from hook_nodeapi and #310212: Remove $op from hook_user: Documentation
currently only block.module has been done, and I did a dirty update to the api docs.
Comment | File | Size | Author |
---|---|---|---|
#48 | block_example.module.patch | 6.59 KB | alexanderpas |
#38 | no_op_in_hook_block_addendum.patch | 13.12 KB | alexanderpas |
#33 | no_op_in_hook_block_addendum.patch | 13.12 KB | alexanderpas |
#31 | no_op_in_hook_block_addendum.patch | 13.12 KB | alexanderpas |
#27 | no_op_in_hook_block_part2.patch | 83.3 KB | alexanderpas |
Comments
Comment #1
alexanderpas CreditAttribution: alexanderpas commentedhere it is...
Comment #2
alexanderpas CreditAttribution: alexanderpas commentedimproved docs some more, also changed block.admin.inc
Comment #3
alexanderpas CreditAttribution: alexanderpas commentedwhy do i seem to forget the patch???
Comment #5
Dave ReidWe Currently have a testing bot failure. Hold on until it's fixed.
Comment #6
alexanderpas CreditAttribution: alexanderpas commentedhopefully i now get a working slave #4 :)
Comment #7
Dries CreditAttribution: Dries commentedYes, please! :)
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commentedsubscribe. #257032: Split block $ops into separate functions has stalled, and this would be a good first step.
Comment #10
alexanderpas CreditAttribution: alexanderpas commentedall tests should pass now.
Comment #12
alexanderpas CreditAttribution: alexanderpas commented7659 passes, 0 fails, and 0 exceptions.
Comment #13
drewish CreditAttribution: drewish commentedsubscribing
Comment #15
alexanderpas CreditAttribution: alexanderpas commentedsome small php error in the api documnetation... thanks botty!
Comment #17
alexanderpas CreditAttribution: alexanderpas commentedsomeone can find that syntax error i made?
Comment #18
Dave ReidFind it hard to believe this patch worked for you. :) When I applied it, all my blocks dissappeared. I went to admin/build/blocks and there were no blocks listed, so obviously something is not right. I tried to go to admin/build/testing but I got the following errors:
Parse error: syntax error, unexpected '}', expecting ')' in /home/davereid/Projects/drupal-head/modules/user/user.test on line 202
Parse error: syntax error, unexpected T_VARIABLE, expecting ';' or '{' in /home/davereid/Projects/drupal-head/modules/user/user.test on line 218
EDIT: Clearing my caches worked for making the blocks reappear in admin/build/blocks. I'm going to check if this works when running upgrade.php after patching.
Comment #19
Dave ReidHere's the patch without the syntax errors.
Comment #20
Dave ReidThere are also some other patches (looks like the do-not-delete-user-1 patch) mixed in with this patch that do not belong. The 'syndicate', 'powered by drupal' and 'recent comments' blocks (node_block and system_block) have not been converted in this patch yet.
Comment #21
alexanderpas CreditAttribution: alexanderpas commentedthanks for finding the mixup i made... this is the patch that should be the one attatched...
Comment #22
Dave ReidVery nice work alexanderpas so far! Marking as code needs works as there is still a lot to complete:
- Convert book_block
- Convert comment_block
- Convert node_block
- Convert poll_block
- Convert profile_block
- Convert search_block
- Convert system_block
- Provide upgrade path? Even just something like:
Adding that function to block.install helped the upgrade path and didn't make all my blocks disappear if I ran update.php after the patch from #21.
Comment #23
alexanderpas CreditAttribution: alexanderpas commentedthanks for that, will be updating those modules, and adding the update code.
also, will be adding some tests for those blocks ;)
Comment #24
alexanderpas CreditAttribution: alexanderpas commentedhere we go, first patch with those modules converted... (let's see what botty makes of it.)
Comment #25
Anonymous (not verified) CreditAttribution: Anonymous commentednice work. some nitpicks follow:
get rid of 'the':
this sentence doesn't make sense:
do whatnow to davereid? ;-)
i think this docblock above book_block_list , book_block_save and book_block_configure can go:
actually, there's a lot of repeated docblocs that need to go.
i don't think the new test in system.test adds much beyond what's in block.test. what would be a valuable addition is to implement the TODO in block.test:
Comment #26
alexanderpas CreditAttribution: alexanderpas commentedadded some more tests, lessened the system.test, added regression tests (tests weither each block from a module is availble.) ;)
Comment #27
alexanderpas CreditAttribution: alexanderpas commentedslightly improved docblocks, full review please?
Comment #29
alexanderpas CreditAttribution: alexanderpas commentedComment #30
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!
Comment #31
alexanderpas CreditAttribution: alexanderpas commentedforgot to convert two modules, statistics and aggregrator, those are done now.
tests in those two modules: 331 passes, 0 fails, and 0 exceptions.
Comment #32
Anonymous (not verified) CreditAttribution: Anonymous commentedlooks good, one nitpick:
i know it was there before, but can we change
<>
to!=
?Comment #33
alexanderpas CreditAttribution: alexanderpas commenteddone!
Comment #34
Anonymous (not verified) CreditAttribution: Anonymous commentedlooks good, all tests pass, RTBC.
Comment #35
drewish CreditAttribution: drewish commentedjustinrandell, i'd be hesitant to change the comparison operator with out testing this on postgresql first. i've gotten bug reports in some of my contrib modules in the past about it.
lets either drop that change out for now and go back to the patch in #31, or get someone to test this on pgsql.
Comment #36
Dave ReidThe '!=' operator will fail on PostgreSQL and is not acceptable by SQL-99. Please leave it as '<>'.
Comment #37
Dave ReidComment #38
alexanderpas CreditAttribution: alexanderpas commentedsame patch as #31
Comment #39
Anonymous (not verified) CreditAttribution: Anonymous commentedahem, sorry for the noise.
Comment #40
Dries CreditAttribution: Dries commentedCommitted to CVS. Thanks for the follow-up.
Comment #41
drewish CreditAttribution: drewish commentedUh someone needs to write some update docs: http://drupal.org/node/224333
Comment #42
drewish CreditAttribution: drewish commentedturns out this was a duplicate of #257032: Split block $ops into separate functions but this one got committed...
Comment #43
Anonymous (not verified) CreditAttribution: Anonymous commentednope, the refactoring issue is not a dupe - it aims for much bigger changes to the block system.
just needs to be brought back up to date.
Comment #44
alexanderpas CreditAttribution: alexanderpas commentedadded update documentation.
Comment #45
alexanderpas CreditAttribution: alexanderpas commentedlast thing to update: block_example.module
Comment #46
RobLoachI think we should really save the kittens here and split the #257032: Split block $ops into separate functions up into a number of separate issues. Removing $op was definitely the first step. Nicely done, everyone!
Comment #47
dropcube CreditAttribution: dropcube commented#351667: DX: Remove hook_block_configure & hook_block_save
Comment #48
alexanderpas CreditAttribution: alexanderpas commentedblock_example.module updated!
Comment #49
alexanderpas CreditAttribution: alexanderpas commentedComment #50
Dries CreditAttribution: Dries commentedThere is no block_example.module in core :)
Comment #51
alexanderpas CreditAttribution: alexanderpas commented@Dries
http://api.drupal.org/api/file/developer/examples/block_example.module/7
Comment #52
drewish CreditAttribution: drewish commentedalexanderpas, that's actually not part of core. it's in contrib so any developer with a cvs account can modify it: http://cvs.drupal.org/viewvc.py/drupal/contributions/docs/developer/exam...
Comment #53
alexanderpas CreditAttribution: alexanderpas commentedstill, it needs to be updated... and i've got no CVS (don't want it yet!)
Comment #54
RobLoachAdd the documentation to block.api.php? Looks like there's already an exciting and amazing example in there :-P .
Comment #55
catchYes let's add the extra detail to hook_block() in block.api.php, then we could probably do away with block_example.module entirely for 7.
Comment #56
alexanderpas CreditAttribution: alexanderpas commentedComment #57
rfayComment #58
rfayAs far as I can tell from a quick review, the goals that were agreed upon in #55 have been met. block.api.php is updated with the 5 current hooks and no reference anywhere to $op.
I recommend that we close this one. I don't believe there's any more work to do unless it's to remove block_example.module as had been proposed in #55.
Comment #59
rfaySince it looks good to me, marking fixed.
Comment #61
jhodgdonI just applied the patch from #47, with a few small modifications, to the contrib repository, to update the block example module for Drupal 7. http://drupal.org/cvs?commit=270062