@buddy: I think we should cleanup the module regarding drupal code style rules first and then reroll D6 patch in http://drupal.org/node/165397.

I will do this cleanup if you hold up other patches.

CommentFileSizeAuthor
#10 googleanalytics_codestyle_1.patch12.96 KBquicksketch
#9 google_analytics.zip_.patch3.39 KBhass
#5 googleanalytics_codestyle_0.patch12.64 KBquicksketch
#4 EclipseScreenSnapz001.png43.33 KBquicksketch
#1 googleanalytics_codestyle.patch13.13 KBhass

Comments

hass’s picture

hass commented
Status: Active » Needs review
StatusFileSize
new googleanalytics_codestyle.patch13.13 KB

Here is the patch that hopefully fixes all code style issues.

hass’s picture

hass commented

Additional that patch adds a new and missing variable to the uninstall hook.

budda’s picture

budda
Primary language English
Location Cheshire
commented
Status: Needs review » Needs work

Seem to be having real trouble applying patches!

This is against the code checked in to CVS yesterday evening.

# patch < googleanalytics_codestyle.patch
(Stripping trailing CRs from patch.)
patching file googleanalytics.module
Hunk #1 FAILED at 8.
Hunk #2 FAILED at 15.
Hunk #3 FAILED at 31.
Hunk #4 FAILED at 41.
Hunk #5 FAILED at 175.
Hunk #6 FAILED at 205.
6 out of 6 hunks FAILED -- saving rejects to file googleanalytics.module.rej
(Stripping trailing CRs from patch.)
patching file googleanalytics.install
Hunk #1 FAILED at 5.
Hunk #2 FAILED at 21.
Hunk #3 FAILED at 31.
3 out of 3 hunks FAILED -- saving rejects to file googleanalytics.install.rej
quicksketch’s picture

quicksketch
he/him
commented
StatusFileSize
new EclipseScreenSnapz001.png43.33 KB

hass, make sure you create the patch relative to the selection, not the project when using Eclipse. As someone that uses Eclipse for everything, I know it's easy to miss!

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new googleanalytics_codestyle_0.patch12.64 KB

Here's a reroll with the relative path set.

hass’s picture

hass commented

i tried my patch and it applies... !??? i've tried this now with cygwin's "patch".

D:\InetPub\wwwroot\drupal-contrib-5\modules\google_analytics>patch < googleanalytics_codestyle.patch
(Stripping trailing CRs from patch.)
patching file googleanalytics.module
(Stripping trailing CRs from patch.)
patching file googleanalytics.install
hass’s picture

hass commented

@quicksketch: if i change this setting i loose all path information and you cannot apply multi-directory patches, isn't it? I found an handbook page some time ago but i cannot find it now... there was a description, but as i remember the eclipse defaults are perfect any the patch applies...

budda’s picture

budda
Primary language English
Location Cheshire
commented

Attempting to apply against $Id: googleanalytics.module,v 1.14.2.10 still fails.
Using ubuntu/debian patch command.

root@leopard:/var/www/html/drupal-5.0/sites/all/modules/google_analytics# patch < googleanalytics_codestyle_0.patch
patching file googleanalytics.module
Hunk #1 FAILED at 8.
Hunk #2 FAILED at 15.
Hunk #3 FAILED at 31.
Hunk #4 FAILED at 41.
Hunk #5 FAILED at 175.
Hunk #6 FAILED at 205.
6 out of 6 hunks FAILED -- saving rejects to file googleanalytics.module.rej
patching file googleanalytics.install
Hunk #1 FAILED at 5.
Hunk #2 FAILED at 21.
Hunk #3 FAILED at 31.
3 out of 3 hunks FAILED -- saving rejects to file googleanalytics.install.rej
hass’s picture

hass commented
StatusFileSize
new google_analytics.zip_.patch3.39 KB

i cannot say what's broken on your machine, but here are the patched files in a zip. remove the ".patch" and extract...

quicksketch’s picture

quicksketch
he/him
commented
StatusFileSize
new googleanalytics_codestyle_1.patch12.96 KB

@hass, you can maintain path information by right-clicking on the directory you wish the patch to be relative from. Select Team->Create Patch from the contextual menu. Then choosing 'Selection' like the above shot makes the patch relative to whatever directory you right-clicked on. If you use the default settings, the people applying the patch must set patch to ignore several parent directories. If it works for you as-is though, maybe I'm wrong and it doesn't cause any problems :)

Gar! I can't believe these patches aren't applying! I can use every patch on this page just fine (if I ignore the directories set in hass's patches). Here's yet another one that's rolled from the command line.

hass’s picture

hass commented

Please add me to the maintainers list to get this bugs fixed. thx

hass’s picture

hass commented
Assigned: Unassigned »
Status: Needs review » Fixed
Anonymous’s picture

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