jtv | Hi wgrant, hi lifeless | 02:17 |
---|---|---|
wgrant | Morning jtv! | 02:18 |
jtv | Not a very busy channel these days, is it? | 02:34 |
lifeless | o/jtv | 02:35 |
=== mapreri_ is now known as mapreri | ||
=== bigjools_ is now known as bigjools | ||
wgrant | cjwatson: On bmp-webhooks, what's the motivation behind *_git_ref? | 23:18 |
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:19 |
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:20 |
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:21 |
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:22 |
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:25 |
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:26 |
cjwatson | Oh. Helpful. Blame leads me back to r5608.7.5, whose commit message is "Further updates". | 23:30 |
wgrant | Heh | 23:31 |
cjwatson | Trunk-only blame is r6060, which added MP notifications. | 23:31 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!