Closed (fixed)
Project:
Drupal.org CVS applications
Component:
new project application
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
29 Jul 2009 at 18:35 UTC
Updated:
2 Oct 2019 at 11:14 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
PeteS commentedComment #2
PeteS commentedErr, had to fix a styling issue.
Comment #3
PeteS commentedScreenshot attached.
Comment #4
AjK commentedComment #5
PeteS commentedReplaced tabs with spaces.
Comment #6
avpadernoThere is no need to define an install function that doesn't do anything.
Comment #7
PeteS commentedFair enough. Empty .install function/file removed.
Comment #8
dave reidSome minor nitpicks:
- I'd remove the
package = User interfacefrom the .info since you are not part of a larger module package like Views or CCK.- Make sure to also remove the last four lines from the .info file as that is going to be added by the drupal.org packaging script automatically, so you should not have those lines in the CVS versions.
- Your access controls on the JS callback should really check user_access('administer menu') and not 'access content' since you are working with menus. This also matches the permission used for actions in menu.module.
Otherwise I think everything looks really really good. Very nice work with this.
-
Comment #9
PeteS commentedAll great suggestions, thank you Dave. Those changes are in the latest tarball.
Comment #10
AjK commentedComment #12
avpaderno