hi,
i did some work fixing all coder warnings and figuring out how this thing works.. did't yet get it to totally work but.. anyways i just wanted to submit this patch so you can cherry pick the fixes and to prevent duplicate effort... there are some debug statements left in and some functions have been renamed, also the JS file lowercased.. cheers.

CommentFileSizeAuthor
ajax-forms-cleanup.patch7.36 KBeMPee584

Comments

brendoncrawford’s picture

brendoncrawford commented

eMPees584,

I understand the reasoning behind some of the changes, but not others. Specifically, what is the reasoning for renaming a number of the functions?

eMPee584’s picture

eMPee584 commented

making them more descriptive of what they do?

brendoncrawford’s picture

brendoncrawford commented
Priority: Normal » Minor
Status: Needs review » Fixed

I have ported in segments of the code. The fixes will be available in the next dev release which will be available in about 12 hours.

eMPee584’s picture

eMPee584 commented
Priority: Minor » Normal
Status: Fixed » Active

Despite the renaming of functions to more sensible (is there such a thing as an ajax type?) names (and the lowercasing of the JS file) you also skipped uppercasing of all constants and removing the ?> from the end of the file. Furthermore, the leading <?php is lowercase in *all* other modules (uppercase it even breaks syntax highlighting f.e. in Kate). Also, the drupal style checker script still complains about no mixed case function or variable names, use lower case and _ :.. another reason why i renamed these functions.

brendoncrawford’s picture

brendoncrawford commented
Assigned: Unassigned » brendoncrawford
Priority: Normal » Minor

1. The module name is "Ajax Forms". "Types" refer to the types of content that are being processed. The concept of sensible function names is somewhat subjective. We will probably just have to agree to disagree on this one.

2. I believe that class names should always be uppercased, and that class files should always reflect the classname. Hence "Ajax.js" reflects the class "Ajax" which reflects the name of the module. If there is a documented Drupal standard that contradicts this, please let me know and I will change it.

3. I have now lowercased the leading php tag.

4. I have now removed the trailing php tag.

5. There have been no mixed cased function names in the module since October 17.

A new dev release will be available within 12 hours.

brendoncrawford’s picture

brendoncrawford commented
Status: Active » Fixed
Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.