Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filter_file option #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

intosi
Copy link

@intosi intosi commented Sep 18, 2013

Add filter_file option. The value gets passed directly to rsync using the
--filter='merge /path/to/filter/file' directive.

Test-Information:

Unit tests pass.
Tested extensively on my local backup machine.

Add filter_file option.  The value gets passed directly to rsync using the
--filter='merge /path/to/filter/file' directive.

Test-Information:

Unit tests pass.
Tested extensively on my local backup machine.
@bebehei bebehei modified the milestones: rsnapshot 1.4, rsnapshot 2.0 Mar 19, 2015
@bebehei
Copy link
Contributor

bebehei commented Mar 23, 2015

Thanks for the PR.

The code in rsnapshot-program.pl looks good. There had been no need to add much logic for filter-file. The rsnapshot.1 has to get removed out of the changeset and probably some tests are missing. Besides this, the PR looks good.

Now my personal opinion: TBH this is a bit of overengineering rsnapshot. I've always seen rsnapshot as a simplification of rsync to do backups and this PR is giving rsnapshot more complexity. Many people don't even understand rsyncs exclude-arguments as you can see on rsnapshot-discuss. I'm not sure yet, which way to go.

I'll let the PR open and decide later which way to go.

@bebehei bebehei added the idea label Mar 27, 2015
@nkadel
Copy link

nkadel commented Jun 18, 2015

filterfile can be really, useful to update the configurations out of band with the basic rsnapshot configuration. It can also make a much more detailed and refined inclusion or exclusion list than --exclude or --include, due to the maximum line length limitations of most operating systems.

@MTecknology
Copy link

@bebehei The changes seem relatively trivial and clean. I don't imagine a large number of people would use this feature, but I can think of a few scenarios where it'd be useful. The people I picture using it /should/ know how that option works within rsync. Is there any chance you could weigh in your opinion as of today?

@intosi (depending on the response) Is there any chance you could take a look at resolving the merge conflicts and also add a few tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants