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/...

Claimed by Corsix.

Basic functionality is converted, needs more work though.

Attachment	Size
faq_ask_d6.patch	16.23 KB

Wow that was fast. How is the documentation going?

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

Attachment	Size
faq_ask_d6_2.patch	21.53 KB

Looking good.

A large number of fixes to coding style were made during the porting process. Updating to the new standards is a perfect time to get a refresher on all the standards which didn't change!

This is interesting since the Coder module shows no standards violations.

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

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).

Single quotes strings are prefereable to double quoted, where possible.Again, control structures.
Again, trailing spaces.

Hmm... Maybe I should let the Coder maintainers know.

Thanks again for doing this.

BTW, you probably should assign this to yourself.

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.

I sent him an email with a screenshot of a failed patch run. The patch looks good to me, but failed. The documentation is pretty good.

BTW, he's listed as UK.

@corsix: It looks like you might have started with the beta4 official release rather than the more current -dev release which has had several more commits made to it.

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

Attachment	Size
faq_ask_d6_3.patch	21.52 KB

Made some changes needed from moving D6RC2 -> D6CVS

Attachment	Size
faq_ask_d6_4.patch	21.71 KB

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.

After I added some blank lines at the end of each section, it applied. Now to some testing...

Hmm... I added a new user and gave her all permissions. She is not being shown in the expert's list. I think thsi also exists in D5 - "authenicated users" are not actually assigned that rid.

I am also not getting an "Ask a question" menu item.

Entering http://d6test/faq_ask gets access denied.

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).

@Angie: I understand what you're saying but this is not a good practice; I've never encountered any IT organizations that allow such things. (I am a professional, certified project manager.) Standards should be hard rules that are approved by some form of approval body (even if that's only one person) after appropriate discussion. If any "Joe Drupal" may change the standards any time he/she feels like it, we will have a shambles and there is no way Doug, or any developer, would be able to keep up.

1. If it's not officially approved, it cannot be documented.
2. If it's not officially documented, it cannot be a standard.

So, having stated my piece, this is really not the appropriate issue to be discussing this.

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.

Attachment	Size
faq_ask_d6_5.patch	21.73 KB

Yes, that fixed both of those problems. I am still testing. I have found some silly little things that have always been there. And I fixed the "authenticated user" expert problem.

The "url" and "l" functions have changed: http://drupal.org/node/114774#url

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

Attachment	Size
faq_ask_d6_6.patch	22.25 KB

Thanks. I'll do some more testing. Please don't forget the handbook page.

Any ideas on this:

# user warning: Duplicate entry '0-5' for key 1 query: INSERT INTO term_node (nid, vid, tid) VALUES (5, 5, 0) in C:\www\drupal-6\modules\taxonomy\taxonomy.module on line 690.
# user warning: Duplicate entry '1-5' for key 1 query: INSERT INTO term_node (nid, vid, tid) VALUES (5, 5, 1) in C:\www\drupal-6\modules\taxonomy\taxonomy.module on line 690.

This looks like it might be a core bug http://drupal.org/node/213591

As soon as the "url" changes get posted to the handbook page, I'll mark this RTBC.

Oops, watchdog changes: http://drupal.org/node/114774#watchdog_parameters

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.

Attachment	Size
faq_ask_d6_7.patch	23.21 KB

Thank you, Peter. How are your other GHOP's doing?

Okay, I can't find any more problems that are within your realm. Thanks a bunch, Peter.

PS: want to convert any more modules? I have several.

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.

6.x-1.x-dev has now been created.

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