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

Only ignore existing matches if preserve_all_lines is used #1858

Merged
merged 1 commit into from
Jul 23, 2014
Merged

Only ignore existing matches if preserve_all_lines is used #1858

merged 1 commit into from
Jul 23, 2014

Conversation

vohi
Copy link
Contributor

@vohi vohi commented Jul 22, 2014

That option is documented to disable convergence, while preserve_block
should respect existing occurances of the promiser within the selected
region.

Redmine #1525

@cfengine-review-bot
Copy link

Can one of the admins verify this patch?

methods:
"any" usebundle => dcs_check_diff("$(G.testfile).actual",
"$(G.testfile).expected",
"$(this.promise_filename)");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation skew. Did you use tabs ?

@kacf
Copy link
Contributor

kacf commented Jul 23, 2014

Code seems good.

That option is documented to disable convergence, while preserve_block
should respect existing occurances of the promiser within the selected
region.

Redmine #1525
@vohi
Copy link
Contributor Author

vohi commented Jul 23, 2014

Updates as per comments. Treating Windows explicitly, as using the printf version makes the test fail, probably due to subtle differences in quoting/newline treatment that I can't quite figure out.

kacf added a commit that referenced this pull request Jul 23, 2014
Only ignore existing matches if preserve_all_lines is used
@kacf kacf merged commit 518f8b5 into cfengine:master Jul 23, 2014
@kacf
Copy link
Contributor

kacf commented Jul 23, 2014

I didn't test Windows right now, it might still fail, but at least the solution shouldn't be far away. Will get to it in the next round of Windows tests.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants