[02:17] <jtv> Hi wgrant, hi lifeless
[02:18] <wgrant> Morning jtv!
[02:34] <jtv> Not a very busy channel these days, is it?
[02:35] <lifeless> o/jtv
[23:18] <wgrant> cjwatson: On bmp-webhooks, what's the motivation behind *_git_ref?
[23:19] <cjwatson> 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] <wgrant> cjwatson: We don't issue ObjectModifiedEvents on supersede, do we?
[23:20] <wgrant> Unless we do, they can't change at all.
[23:20] <cjwatson> wgrant: We do now, I fixed that.
[23:20] <wgrant> Oh.
[23:20] <wgrant> Hm, that's inconvenient.
[23:20] <cjwatson> (back in consistent-bmp-events or so)
[23:20] <cjwatson> That said, supersede would be a "modify old, create new" pair
[23:21] <wgrant> I'd prefer that we special-cased those fields to emit both when either changed.
[23:21] <wgrant> Oh
[23:21] <wgrant> Apart from git_repository.branches and snap.git_ref, git_ref is input-only.
[23:21] <wgrant> BMP never exposes them on the API today
[23:21] <cjwatson> We issue an OME to describe the state change to Superseded, is all.
[23:21] <wgrant> Oh
[23:21] <cjwatson> (Which means that now people actually get mail about that)
[23:21] <wgrant> So the fields can't appear to change in an event?
[23:22] <wgrant> That's what I suspected would be the case.
[23:22] <cjwatson> Yeah, I wonder why these are in BMPDelta to begin with
[23:25] <cjwatson> 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] <wgrant> That's certainly my preference.
[23:25] <wgrant> It seems very weird to have them there.
[23:25] <cjwatson> I probably wouldn't have started down that path had *_branch not been in BMPDelta.delta_values to begin with.
[23:26] <wgrant> Yep
[23:26] <wgrant> It gives us more flexibility with the payload composition.
[23:26] <wgrant> And makes it slightly less strange.
[23:26] <wgrant> (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] <cjwatson> Oh.  Helpful.  Blame leads me back to r5608.7.5, whose commit message is "Further updates".
[23:31] <wgrant> Heh
[23:31] <cjwatson> Trunk-only blame is r6060, which added MP notifications.