[02:17] Hi wgrant, hi lifeless [02:18] Morning jtv! [02:34] Not a very busy channel these days, is it? [02:35] o/jtv === mapreri_ is now known as mapreri === bigjools_ is now known as bigjools [23:18] cjwatson: On bmp-webhooks, what's the motivation behind *_git_ref? [23:19] wgrant: I think it's cleaner to give a unified description of changes in any of the refs being used; a change in the repository alone isn't very meaningful, say [23:19] cjwatson: We don't issue ObjectModifiedEvents on supersede, do we? [23:20] Unless we do, they can't change at all. [23:20] wgrant: We do now, I fixed that. [23:20] Oh. [23:20] Hm, that's inconvenient. [23:20] (back in consistent-bmp-events or so) [23:20] That said, supersede would be a "modify old, create new" pair [23:21] I'd prefer that we special-cased those fields to emit both when either changed. [23:21] Oh [23:21] Apart from git_repository.branches and snap.git_ref, git_ref is input-only. [23:21] BMP never exposes them on the API today [23:21] We issue an OME to describe the state change to Superseded, is all. [23:21] Oh [23:21] (Which means that now people actually get mail about that) [23:21] So the fields can't appear to change in an event? [23:22] That's what I suspected would be the case. [23:22] Yeah, I wonder why these are in BMPDelta to begin with [23:25] Maybe a better fix is to delete those *and* their *_branch partners from BMPDelta.delta_values (with a bit more hunting around to make sure of this) and move the webhook payload composition directly into lp.code.subscribers.branchmergeproposal since once it needs to be a superset of BMPDelta's list it doesn't derive very much benefit from being there [23:25] That's certainly my preference. [23:25] It seems very weird to have them there. [23:25] I probably wouldn't have started down that path had *_branch not been in BMPDelta.delta_values to begin with. [23:26] Yep [23:26] It gives us more flexibility with the payload composition. [23:26] And makes it slightly less strange. [23:26] (I also don't really know why the new_values thing is separate... surely that's a property of the output format, not the delta... but anyway) [23:30] Oh. Helpful. Blame leads me back to r5608.7.5, whose commit message is "Further updates". [23:31] Heh [23:31] Trunk-only blame is r6060, which added MP notifications.