GHOP #160 - Convert Faq_Ask to D6 and Document Steps

The official task associated with this issue is at:
http://code.google.com/p/google-highly-open-participation-drupal/issues/...

Basic functionality is converted, needs more work though.

First draft of handbook page: http://drupal.org/node/212702
Also attached updated patch.

It is odd that coder didn't pick up on them, however some excerpts from the patch file to show what I mean:

-  if ($user->uid == 0) { return null; }
-  
+  if ($user->uid == 0) {
+    return null;
+  }
+

Control strucutres shouldn't be all on one line, trailing spaces should be removed (personally I find it very odd that that trailing spaces is mentioned on http://drupal.org/node/539 and not on http://drupal.org/coding-standards, but that's another matter).

-      $result = db_query("SELECT u.mail FROM {faq_expert} e JOIN {users} u USING (uid) WHERE e.tid=%d", $category);
+      $result = db_query('SELECT u.mail FROM {faq_expert} e JOIN {users} u USING (uid) WHERE e.tid=%d', $category);

Single quotes strings are prefereable to double quoted, where possible.

+  if (count($result) > 0) {
+    drupal_set_message(t('faq_ask module installed.'));
+  }
+  else {
+    drupal_set_message(t('faq_ask table creation failed. Please "uninstall" the module and retry.'));
   }
-
-  if ($result) { drupal_set_message(t('faq_ask module installed.')); }
-  else { drupal_set_message(t('faq_ask table creation failed. Please "uninstall" the module and retry.')); }

Again, control structures.

-        $left = null;  
+        $left = null;

Again, trailing spaces.

There are under 3 hours for students to claim tasks. I assume since Corsix is in Germany, he's sleeping since it's like 3am there. ;)

However, it'd really help nancy, if you could post a summary of what's left to be done on this task on the off-chance he gets up early enough to finish it off so he can claim another. Alternately, if you feel it's working, please say so and we can mark it fixed so he can claim is 15th (and final) task.

This one applies cleanly; http://img102.imageshack.us/img102/9428/image1sk9.png

Made some changes needed from moving D6RC2 -> D6CVS

http://img102.imageshack.us/my.php?image=image1wy8.png is a screenshot of some of my testing, but will continue working on this until it's finished.

I gave this a run-through tonight before the task deadline. I can't speak with certainty whether it's 100% correct, and there are some notices that still need checking that didn't appear due to patching against RC2. HOwever, the bulk of the work looks like it's done, and the handbook page is in good shape. So we've marked this task complete over in the Google tracker.

Leaving this as needs review.

corsix wrote:

"Single quotes strings are prefereable to double quoted, where possible."

Does it say so in the coding standards?

I don't think that this is true. Yes, single quotes take slightly less processing time ot handle, but when quoting SQL, it's better to use double quotes because the ANSI SQL standard uses single quotes ONLY for quoted terms. Thus, it's a lot easier to read a SQL string that is "SELECT * FROM {node} WHERE title LIKE '%something%'" instead of 'SELECT * FROM {node} WHERE title LIKE \'%something%\'' and actually prevents people from mistakenly using double quotes as in 'SELECT * FROM {node} WHERE title LIKE "%something%"'.

corsix also writes:

trailing spaces should be removed (personally I find it very odd that that trailing spaces is mentioned on http://drupal.org/node/539 and not on http://drupal.org/coding-standards, but that's another matter).

... Which is why they aren't checked for by coder, because their not in the coding standards document. I guess checking for trailing spaces wouldn't be terrible, but the use of tabs causes people many more problems than trailing spaces. These are easy to fix though, :%s/ *$//g

We use Drupal core as a benchmark for what the coding standards should be, because that has by far the most eyes/quality control on it. If core does something consistently, then that's the coding standard, regardless if anyone's gotten around to writing it up in the coding standards document "formally" yet.

However, if you see things like that that aren't yet updated (no trailing spaces, etc.) then please feel free to amend the standards accordingly so they reflect the "working" standard.

The coding standards document in CVS does now mention tailing spaces, but that is only periodically copied across to the d.o page (see http://drupal.org/node/205432).

I'm willing to bet that most IT organizations are not open source projects. ;)

But we can discuss the concept of a "gatekeeper" to the coding standards @ the webmasters queue.

In the previous patch, an extra space somehow got into the menu access arguments key for the faq ask page, leading to the page access problem.

Not sure how I missed those, but they are updated now :)

Updated patch; hook_menu no longer calls t(), as it is done automatically by menu, calls to watchdog() no longer use t(), as it is done automatically.

Updated documentation page; additions and corrections.

Another one would be good (hopefully I'll do more of these changes in the 1st iteration), but I'd want to get my final GHOP out of the way first.