-
Notifications
You must be signed in to change notification settings - Fork 6
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
make lsyncd delete-option configurable #7
Open
wbob
wants to merge
7
commits into
negz:master
Choose a base branch
from
wbob:delete-option
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
30b18a9
add delete option
wbob 616a4f6
fixed option to use boolean values instead of string
wbob ef029a1
include delete option only if false
wbob 75d3e1e
clarify README.md
wbob dc15b6e
implement the possible delete parameters beyond boolean false
wbob 60773c3
fix typo
wbob 44add03
bump version to 1.2.3
wbob File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
$targetdir, | ||
$host, | ||
$ensure = present, | ||
$delete = true, | ||
$rsync_options = {}, | ||
$ssh_options = {}, | ||
) { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I really appreciate you taking the time.
The only part I can't quite follow is this block (and the similar block in
rsyncssh.conf.lua.erb
). Is there a reason we can't just adddelete = <%= @delete %>
to the templates without the surrounding conditionals?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Nic - I appreciate your willingness to look into it.
The reason for the first conditional was to cause no change in config files for existing configurations. As
true
is the default I opted to not print it then.As for the second conditional, reading the doc it seems to me the first two options are boolean, the former ones string.
I just tested to include the modes
running/startup
unquoted and they do not follow their advertised behaviour - i.e.startup
is not supposed to delete on target during runtime, but will do so if the setting is placed unquoted (as it resolves tobool true
?). Same goes for printingtrue
quoted, it will not delete on target. Maybe I should look into lsyncd if they can all be made strings.If the control structure / indentation looks awkward or introduces confusion and you prefer any other way, please change. To me the distinction boolean/string-option proved to be important and therefore at least one conditional separating the option types is needed.