Re-port D6 version to D7 [#977502]

Per deekayen's request I've ported a fresh copy of the D6 dev branch to D7. Due to the lack of a stable release of simpletest I haven't touched the .test file.

Comment	File	Size	Author
#2	masquerade-7.x-dev-1.patch	47.9 KB	afreeman
#2
	masquerade-7.x-dev-0.patch	47.61 KB	afreeman

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

deekayen CreditAttribution: deekayen commented 20 November 2010 at 18:32

Status:

Needs review

» Needs work

Just a few little things I saw, and then I'll probably commit it so people can just start testing and reporting bugs. Simpletest 7.x is in core - that's what http://qa.drupal.org/ is all about. It'd be nice to have the tests updated where applicable.

The @file in .install shouldn't have new whitespace introduced on the line.
I know coder will complain about the removal of the comma on .install line ~43
The update functions - you removed the "Implements..." on one, but kept the comment and did the opposite on the other. IIRC, the correct one is the single line sentence summary, not "Implements..."
Coder 7.x complains when any function isn't documented (e.g. docblock for masquerade_admin_settings())
I'm just curious - why does system_settings_form() need to be before the additional submission handlers now?

Comment #2

afreeman CreditAttribution: afreeman commented 21 November 2010 at 23:39

File	Size
masquerade-7.x-dev-1.patch	47.9 KB

Thanks for getting a review together so quickly. A lot of what you've mentioned is cruft that came out of the 6.x branch when I checked it out. I've changed what you mentioned and re-ran coder set to minor, no additional issues reported.

As far as the timing goes for system_settings_form() (et al) a lot of this code represents a best guess on my part, especially in the area of the hook_user() code. All I can say for sure is the patched version successfully masquerades in my development environment. I'm looking forward to getting additional eyes on this for testing/bug reports.