Here's a review I put on the MP but I'll put it here for posterity:
Hey there! Thanks for raising this much, much appreciated. This is great as-is but here are some points that'd make it EVEN better: - https://git.launchpad.net/~sudipmuk/ubuntu/+source/zeitgeist/commit/?id=95fd328c45f72fa0f3895900216ad0726e86139e -> it misses the DEP-3 headers. I am aware that you're just rebasing the new version and you're not really adding the patch itself. But whilst at it, if you could really add DEP-3 headers to a patch, that'd be EXCELLENT! See: https://git.launchpad.net/~sudipmuk/ubuntu/+source/zeitgeist/commit/?id=a6cff2e5ac675b90cc574a26dc953f452aa9a00c for example. It has the DEP-3 headers and tells me why it's needed, some bug history, et al. I generally look at it because sometimes we shouldn't really be merging stuff as-is and carry-forwarding the tech-debt but actively be looking at drop the delta wherever and whenever possible. Now, OTOH, look at https://git.launchpad.net/~sudipmuk/ubuntu/+source/zeitgeist/commit/?id=e940d9763d0fba97dc8504ed382e01210a43f08d -> it gives me a quick description but doesn't tell me if it's coming from upstream or was it written by downstream, is there an upstream bug? a downstream bug? or? and I spend my time looking at things, et al. That's why DEP-3 headers are really useful. - https://git.launchpad.net/~sudipmuk/ubuntu/+source/zeitgeist/commit/?id=2ab99989a22d2e07bfc28e5eae942bd8283c1aa7 -> did you verify if that's still needed? why? :) So except for the above question and a general pointer (first point), it mostly looks good! :D -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to zeitgeist in Ubuntu. https://bugs.launchpad.net/bugs/2044300 Title: Please merge zeitgeist 1.0.4-5 (universe) from Debian unstable (main) Status in zeitgeist package in Ubuntu: Confirmed Bug description: Please merge zeitgeist 1.0.4-5 (universe) from Debian unstable (main) Explanation of the Ubuntu delta: Carry forward three previous Ubuntu specific patch. 1. add_datahub_autostart_delay.patch 2. nodisplay_autostart.patch 3. pre_populator.patch Extra change for this version: The monitor-test was skipped in the previous version as it was failing. That test has now been enabled in Debian but is still failing in Ubuntu. And, so the extra change for this version was to disable it for now. To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/zeitgeist/+bug/2044300/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
