Re: [PATCH 1/2] Add per-svn-remote ignore-paths config
From: Eric Wong <hidden>
Date: 2016-06-15 22:46:35
Ben Jackson [off-list ref] wrote:
The --ignore-paths option to fetch is very useful for working on a subset
of a SVN repository. For proper operation, every command that causes a
fetch (explicit or implied) must include a matching --ignore-paths option.
This patch adds a persistent svn-remote.$repo_id.ignore-paths config by
promoting Fetcher::is_path_ignored to a member function and initializing
$self->{ignore_regex} in Fetcher::new. Command line --ignore-paths is
still recognized and acts in addition to the config value.
Signed-off-by: Ben Jackson <redacted>Hi Ben, the patch looks useful, but there are some minor issues with comments inline.
quoted hunk ↗ jump to hunk
--- a/git-svn.perl +++ b/git-svn.perl # return value: 0 -- don't ignore, 1 -- ignore sub is_path_ignored { - my ($path) = @_; + my ($self, $path) = @_; return 1 if in_dot_git($path); + return 1 if defined($self->{ignore_regex}) && + $path =~ m!$self->{ignore_regex}!o;
Since we're making the regex per-remote, I'd remove the "o" operator. It can break things if Git::SVN::Fetcher ever gets reused by multiple SVN remotes within the same process.
quoted hunk ↗ jump to hunk
--- a/t/t9134-git-svn-ignore-paths.sh +++ b/t/t9134-git-svn-ignore-paths.sh@@ -44,7 +51,7 @@ test_expect_success 'SVN-side change outside of www' ' test_expect_success 'update git svn-cloned repo' ' ( cd g && - git svn rebase --ignore-paths="^www" && + git svn rebase &&
I'd rather not change the existing tests since existing behavior can break. Instead, augment the existing tests so we test both the .git/config and command-line cases. Thanks. -- Eric Wong