Code review comment for ~sudipmuk/ubuntu/+source/zeitgeist:merge-lp2044300-noble

Hi Utkarsh,

Thanks for looking into this.

On Sat, 25 Nov 2023 at 17:04, Utkarsh Gupta
<email address hidden> wrote:
>
> Review: Needs Information
>
> 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!

I could not find any bug report which might explain this patch. Seems
to be an Ubuntu specific patch and introduced in
zeitgeist_0.9.14-0ubuntu1.

>
> 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.

This one seems to fix LP: #962265 but I could not find a way to be
sure and so not confident enough to add DEP-3 headers to the patch.
Again, this also was introduced in zeitgeist_0.9.14-0ubuntu1.

>
> - https://git.launchpad.net/~sudipmuk/ubuntu/+source/zeitgeist/commit/?id=2ab99989a22d2e07bfc28e5eae942bd8283c1aa7 -> did you verify if that's still needed? why? :)

Yes, this is still needed. My initial build failed as I did not have
this one disabled. I have done a diff of the buildinfo and saw there
are version mismatch of many packages between Debian and Ubuntu. One
of the mismatch is still causing the issue in Ubuntu, but I don't know
which one yet.

--
Regards
Sudip