Code review comment for lp:~talex5/ubuntu/precise/zeroinstall-injector/fix-for-953756

Revision history for this message
James Page (james-page) wrote :

Hi Thomas

Thanks for taking the time to prepare this fix.

Some feedback:

1) debian/changelog.

You don't need to reference the full URL to the bug report: (LP: #953756) is sufficient and will trigger the bug to auto-close when the update is uploaded.

Also the version number should have a ubuntu suffix;

1.4.1-2 -> 1.4.1-1ubuntu1

2) debian/patches/packagekit-api.patch

The patch has some generated information in it still (this should be dropped); it would be great if this fix was forwarded upstream as well and the generated bug reference in a Forwarded: field as described in the generated information. Or if it came from upstream document this using appropriate fields.

I notice that Debian now has version 1.6 - would this resolve this bug? Might make more sense to re-sync if its not to invasive and upgrade.

Cheers

James

review: Needs Fixing

« Back to merge proposal