| Message ID | 20200214225929.541306-1-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | Wildcard matching for credentials | expand |
Hi brian, On Fri, Feb 14, 2020 at 10:59:26PM +0000, brian m. carlson wrote: > This series introduces wildcard matching (that is, urlmatch support) for > credential config options, just like for the http options. This is > helpful in corporate environments where custom credentials should be > used across a wide variety of subdomains. > > In addition, there's an additional test for urlmatch behavior with > multiple subdomains and a mailmap update for the email address used in > this series. I can imagine that this is perhaps for Git LFS, which I could see benefiting from this change. My review has nothing to do with my affiliation (or lack thereof) to LFS. I gave your patches a review, and they all look quite good to me. Thanks especially for 2/3, which I would have suggested were it not already there ;-). This looks good to me, so please have my: Reviewed-by: Taylor Blau <me@ttaylorr.com> > brian m. carlson (3): > mailmap: add an additional email address for brian m. carlson > t1300: add test for urlmatch with multiple wildcards > credential: allow wildcard patterns when matching config > > .mailmap | 1 + > Documentation/gitcredentials.txt | 4 +++- > credential.c | 41 +++++++++++++++++--------------- > t/t0300-credentials.sh | 20 ++++++++++++++++ > t/t1300-config.sh | 6 +++++ > 5 files changed, 52 insertions(+), 20 deletions(-) Thanks, Taylor
On 2020-02-14 at 23:58:22, Taylor Blau wrote: > Hi brian, > > On Fri, Feb 14, 2020 at 10:59:26PM +0000, brian m. carlson wrote: > > This series introduces wildcard matching (that is, urlmatch support) for > > credential config options, just like for the http options. This is > > helpful in corporate environments where custom credentials should be > > used across a wide variety of subdomains. > > > > In addition, there's an additional test for urlmatch behavior with > > multiple subdomains and a mailmap update for the email address used in > > this series. > > I can imagine that this is perhaps for Git LFS, which I could see > benefiting from this change. My review has nothing to do with my > affiliation (or lack thereof) to LFS. It was originally prompted by a discussion that someone started on the Git LFS issue tracker, yes. I pointed out that it's actually Git that controls credential helper matching and so I sent a patch. I've also worked somewhere with multiple internal Git servers, so I can imagine this would also be valuable in such an environment. > I gave your patches a review, and they all look quite good to me. Thanks > especially for 2/3, which I would have suggested were it not already > there ;-). I think there was some discussion on the list about whether allowing multiple wildcards and wildcards in any part of the domain name was intentional or not, and it was decided that it was. I promised to follow up with a patch to the testsuite, and here I am, some time later. > This looks good to me, so please have my: > > Reviewed-by: Taylor Blau <me@ttaylorr.com> Thanks.