Bug#945015: only-branch only takes exact branch names
On Thu, 19 Mar 2020 10:44:06 +, Iain Lane wrote: > > > So, please, Iain, send that merge request. Thanks! > > The merge request exists, thanks Iain! > > https://salsa.debian.org/kgb-team/kgb/-/merge_requests/7 > Ah yeah, I should have updated the bug. No worries. > > To me it looks good, and I especially appreciate the added tests. > > Iain, I guess you tested the changes also with a test installation, > > as discussed on IRC? > I did, yes - and it seems to work as designed. Thanks for the initial > review! Great, thanks for the real-life tests. > Hopefully once this is merged the "production" instances can be updated > to use it? Then I can fix pkg-gnome's webhooks to not spam #debian-gnome > :-). Right, that would indeed make a lot sense :) On Thu, 19 Mar 2020 15:26:17 +0200, Damyan Ivanov wrote: > > Dam, I hope you have a moment to look at the changes as well. > Looks good to me. Thanks! > The only things missing for me are: > > * addition of Iain in the copyright notices and debian/copyright > * addition of the new (build) dependencies to Build.PL > > But I guess these can be made at release time as well. Alright, I merged the patch, updated the copyright notices and Build.PL and did a round of packaging polishing. Dam, could you take another look and maybe release the thing (or I can do it as well if you don't have time)? Cheers, gregor -- .''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06 `. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe `- NP: Don McLean: He's Got You signature.asc Description: Digital Signature
Bug#945015: only-branch only takes exact branch names
-=| gregor herrmann, 19.03.2020 11:34:19 +0100 |=-
> The merge request exists, thanks Iain!
>
> https://salsa.debian.org/kgb-team/kgb/-/merge_requests/7
>
> To me it looks good, and I especially appreciate the added tests.
> Iain, I guess you tested the changes also with a test installation,
> as discussed on IRC?
>
> Dam, I hope you have a moment to look at the changes as well.
Looks good to me.
This:
- ... not grep { ... } @{...}
+ ... not any { } @{...}
is a nice catch :)
The only things missing for me are:
* addition of Iain in the copyright notices and debian/copyright
* addition of the new (build) dependencies to Build.PL
But I guess these can be made at release time as well.
-- dam
Bug#945015: only-branch only takes exact branch names
On Thu, Mar 19, 2020 at 11:34:19AM +0100, gregor herrmann wrote: > On Tue, 26 Nov 2019 21:27:14 +0200, Damyan Ivanov wrote: > > > -=| gregor herrmann, 25.11.2019 21:38:54 +0100 |=- > > > On Mon, 18 Nov 2019 12:42:55 +, Iain Lane wrote: > > > > > > > I think it'd be cool if this were instead to support globbing. If I were > > > > to propose a merge request which changes this into a glob (Text::Glob?), > > > > would you merge that? > > > > > > I think the idea totally makes sense, and if the change is > > > backwards-compatible and doesn't pose any other issue I don't see why > > > we wouldn't take it. > > > > > > Maybe Dam who wrote the webhook support has some ideas on how to best > > > implement it or can share other thoughts. > > > > I think that using Text::Glob is a nice idea. > > > > Reading Text::Glob(3pm) I noticed that '/' is treated specially - '*' > > doesn't match it. Hopefully that's not a deal breaker. > > > > Going for full regular expression support seems overkill, and > > dangerous. > > > > So, please, Iain, send that merge request. Thanks! > > The merge request exists, thanks Iain! > > https://salsa.debian.org/kgb-team/kgb/-/merge_requests/7 Ah yeah, I should have updated the bug. > To me it looks good, and I especially appreciate the added tests. > Iain, I guess you tested the changes also with a test installation, > as discussed on IRC? I did, yes - and it seems to work as designed. Thanks for the initial review! Hopefully once this is merged the "production" instances can be updated to use it? Then I can fix pkg-gnome's webhooks to not spam #debian-gnome :-). Cheers, -- Iain Lane [ i...@orangesquash.org.uk ] Debian Developer [ la...@debian.org ] Ubuntu Developer [ la...@ubuntu.com ] signature.asc Description: PGP signature
Bug#945015: only-branch only takes exact branch names
On Tue, 26 Nov 2019 21:27:14 +0200, Damyan Ivanov wrote: > -=| gregor herrmann, 25.11.2019 21:38:54 +0100 |=- > > On Mon, 18 Nov 2019 12:42:55 +, Iain Lane wrote: > > > > > I think it'd be cool if this were instead to support globbing. If I were > > > to propose a merge request which changes this into a glob (Text::Glob?), > > > would you merge that? > > > > I think the idea totally makes sense, and if the change is > > backwards-compatible and doesn't pose any other issue I don't see why > > we wouldn't take it. > > > > Maybe Dam who wrote the webhook support has some ideas on how to best > > implement it or can share other thoughts. > > I think that using Text::Glob is a nice idea. > > Reading Text::Glob(3pm) I noticed that '/' is treated specially - '*' > doesn't match it. Hopefully that's not a deal breaker. > > Going for full regular expression support seems overkill, and > dangerous. > > So, please, Iain, send that merge request. Thanks! The merge request exists, thanks Iain! https://salsa.debian.org/kgb-team/kgb/-/merge_requests/7 To me it looks good, and I especially appreciate the added tests. Iain, I guess you tested the changes also with a test installation, as discussed on IRC? Dam, I hope you have a moment to look at the changes as well. Cheers, gregor -- .''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06 `. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe `- NP: Penelope Swales: Letter to the Dead signature.asc Description: Digital Signature
Bug#945015: only-branch only takes exact branch names
Hi Iain, gregor, Sorry for the late reply to this. -=| gregor herrmann, 25.11.2019 21:38:54 +0100 |=- > On Mon, 18 Nov 2019 12:42:55 +, Iain Lane wrote: > > > I think it'd be cool if this were instead to support globbing. If I were > > to propose a merge request which changes this into a glob (Text::Glob?), > > would you merge that? > > I think the idea totally makes sense, and if the change is > backwards-compatible and doesn't pose any other issue I don't see why > we wouldn't take it. > > Maybe Dam who wrote the webhook support has some ideas on how to best > implement it or can share other thoughts. I think that using Text::Glob is a nice idea. Reading Text::Glob(3pm) I noticed that '/' is treated specially - '*' doesn't match it. Hopefully that's not a deal breaker. Going for full regular expression support seems overkill, and dangerous. So, please, Iain, send that merge request. Thanks! -- Damyan
Bug#945015: only-branch only takes exact branch names
On Mon, 18 Nov 2019 12:42:55 +, Iain Lane wrote: > I think it'd be cool if this were instead to support globbing. If I were > to propose a merge request which changes this into a glob (Text::Glob?), > would you merge that? I think the idea totally makes sense, and if the change is backwards-compatible and doesn't pose any other issue I don't see why we wouldn't take it. Maybe Dam who wrote the webhook support has some ideas on how to best implement it or can share other thoughts. Cheers, gregor -- .''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06 `. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe `- NP: Aimee Mann: Could've Been Anyone signature.asc Description: Digital Signature
Bug#945015: only-branch only takes exact branch names
Package: src:kgb-bot Version: 1.55-2 Severity: normal Tags: upstream Hey, In the GNOME team we're going to start hosting Ubuntu's packaging branches on salsa soon. To avoid spamming #debian-gnome with Ubuntu notifications, I'd like to make KGB notify only on git commits to branches matching `debian/*`, `upstream/*` and `pristine-tar` to OFTC, and `ubuntu/*`, `upstream/*` and `pristine-tar` to Freenode. (possibly in future also doing this for merge request target branches too - maybe just extending `only_branch` to be considered for MRs as well) It seems that `only_branch` matches exactly (with `eq`) the branches that are given, though - this would mean that I have to list all possible `debian/foo` resp. `ubuntu/foo` branches in the corresponding webhooks and keep updating those as the distros create new releases. I think it'd be cool if this were instead to support globbing. If I were to propose a merge request which changes this into a glob (Text::Glob?), would you merge that? Cheers, -- Iain Lane [ i...@orangesquash.org.uk ] Debian Developer [ la...@debian.org ] Ubuntu Developer [ la...@ubuntu.com ]
