I'd like to help reviewing and merging some ideas (RDF, metadata, XMP work) in with this established module, http://drupal.org/sandbox/dman/1310856
(eg, I already have a data mapping wizard, which finds existing tags in sample uploaded images to assist the field mapping, and am fully supporting XMP with RDF support)

BUT this current code is largely untouchable at the moment due to coding standards inconsistencies.
Here is an automated patch that the D7 grammar parser generated, fixing a load of common whitespace and layout problems.
It's a huge rewrite, so I'd suggest you try doing the same thing with the coder.module and grammar parser on your own machine to generate your own diff rather than use this directly. maybe.

CommentFileSizeAuthor
#11 exif-n1336004-11.patch117.18 KBDamienMcKenna
#10 exif-n1336004-10.patch114.92 KBDamienMcKenna
#8 exif-n1336004-8.patch130.18 KBDamienMcKenna
#8 exif-n1336004-8.interdiff.txt5.33 KBDamienMcKenna
#7 exif-n1336004-7.patch128.15 KBDamienMcKenna
#7 exif-n1336004-7.interdiff.txt24.9 KBDamienMcKenna
#5 exif-n1336004-5.patch113.93 KBDamienMcKenna
#5 exif-n1336004-5.interdiff.txt1.47 KBDamienMcKenna
#4 exif-n1336004-4.patch113.93 KBDamienMcKenna
exif.patch35.5 KBdman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dman’s picture

dman CreditAttribution: dman commented

Also, there is a global misspelling 'retreive' that should be replaced everywhere, in a second process, after the code review is applied

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented
Issue summary: View changes
Status: Needs review » Needs work

There's no way this applies anymore, so I'm marking this as Needs Work.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Parent issue: » #3267867: Plan for EXIF 7.x-1.10
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs work » Needs review
FileSize
exif-n1336004-4.patch113.93 KB

I first ran phpcbf on the codebase and then went through using phpcs to manually fix lots more problems. I have not documented most of the function parameters and return statements, but I tried to at least indicate their types.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
exif-n1336004-5.interdiff.txt1.47 KB
exif-n1336004-5.patch113.93 KB

Some minor fixes for #4.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Assigned: Unassigned » DamienMcKenna

Am working on fixing the tests.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
exif-n1336004-7.interdiff.txt24.9 KB
exif-n1336004-7.patch128.15 KB

More WIP.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
exif-n1336004-8.interdiff.txt5.33 KB
exif-n1336004-8.patch130.18 KB

The tests now pass, but there are some problems with them:

    // @todo Work out why this changed.
    // Old value.
    // $this->assertText('51.2977', 'latitude is correct');
    // New value.
    $this->assertText('51.297699999998', 'latitude is correct');

    // @todo Work out why this changed.
    // Old value.
    // $this->assertText('ich bin ein kleiner kommentar', 'comment is correct');
    // New value.
    $this->assertText('ich bin ein kleiner kommenta', 'comment is correct');

    // @todo Work out why this changed.
    // Old value.
    // $this->assertText('Der Titel', 'title is correct');
    // New value.
    $this->assertText('Der Tite', 'title is correct');

    // @todo This value is rendered differently with Date 7.x-2.12+.
    // $this->assertText('2009/01/23 08:52:43', 'date (with Date module) is correct');

    // @todo Problem 1: The sample.jpg file does not list dates from 2011.
    // @todo Problem 2: The fields end up filled with the current date instead
    // of dates from the file.
    // $this->assertText('2011-11-05T21:39:10+01:00', 'file date (without Date module) is correct');
    // $this->assertText('2011/11/05 21:39:10', 'file date (with Date module) is correct');
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Parent issue: #3267867: Plan for EXIF 7.x-1.10 » #3273804: Plan for Exif 7.x-1.11
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
exif-n1336004-10.patch114.92 KB

Moar fixes.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
exif-n1336004-11.patch117.18 KB

This is pretty complete, the basics are present and now what's left is to add descriptions for all functions, methods and their arguments & returns.

  • DamienMcKenna committed 3207525 on 7.x-1.x 
    Issue #1336004 by DamienMcKenna, dman: Coding standards fixes.
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs review » Fixed

Committed.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Assigned: DamienMcKenna » Unassigned

Status: Fixed » Closed (fixed)

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