Posted by moshe weitzman on September 5, 2006 at 2:38pm
7 followers
Jump to:
| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | documentation |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
see title. is trivial bug fix, so rtbc
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| patch_123 | 433 bytes | Ignored: Check issue status. | None | None |
Comments
#1
I'd call this an API change. I won't commit it without further discussion about the impact on contrib modules being updated.
#2
This would be easy to tell, by searching contrib for '_comment(' and seeing if the modules in question contained .info files. If not, they haven't been ported to HEAD yet. If so, see if they implement the form op.
My guess is the number of contrib modules affected would be next to nothing if this was done soon.
#3
we fixed loads of bugs like this after the feature freeze last time. further, we don't support multiple ways of doing things. i hope webchick's reply is sufficient. please postpone or commit this.
#4
Let's figure out how many modules are using this ...
Why is this a 'bug'?
#5
The last feature freeze isn't the best example a feature freeze we want to repeat.
Webchick just provided instructions for how to research this. Those would need to be acted upon.
This reminds me of the node module patch that removed a bunch of hooks and got reversed.
I think this is an okay patch, but I think it should wait for API development to open up again unless clearly demonstrated otherwise.
#6
we avoid offerring multiple ways of doing same thing. this comment form is inconsistent with every other form in drupal (multiple ways to alter it). thus i call it a bug. others are welcome to call it a shoe or a tree or whatever they please.
are we trying to determine the number of modules that already upgraded to HEAD and kept this code in? that seems to me to be the only interesting number.
a module that uses the comment('form') hook just needs to move same code into its hook_form_alter() and remove a return. hardly onerous.
#7
It is still an API change. No one has done any research into this (only how to do the research).
I'd be willing to commit well-written function documentation saying this is deprecated and alter should be used instead.
#8
still applies, with offset. and still a good idea.
#9
Committed to CVS HEAD. Thanks.
#10
#11
This needs to be documented in the API docs and the update guide.
#12
#13
I just updated the API docs.
#14
Automatically closed -- issue fixed for two weeks with no activity.
#15
*ahem*, finally added to the upgrade docs since I ran into this over at #361649: Issue comments broken in D6 HEAD:
http://drupal.org/node/114774/revisions/view/405000/421906
Also setting the title back for posterity...