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

test Task ReplaceInFile for multiple strings in file #128

Merged
merged 2 commits into from
Mar 15, 2015

Conversation

pscheit
Copy link
Contributor

@pscheit pscheit commented Mar 3, 2015

I noticed that the current implementation allows to supply an array of arguments for to and from, so that IO access to files can be minimized. I think this is a good feature, but I noticed that there is no test for that.
So I created a little test and a sentence in the documentation for this behaviour.

Best regards Philipp

@@ -39,14 +39,19 @@ $this->taskReplaceInFile('config.yml')
->regex('~^service:~')
->to('services:')
->run();

$this->taskReplaceInFile('box/robo.txt')
Copy link
Member

Choose a reason for hiding this comment

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

please move this example to dockblock of the class.
this file is generated from it

@pscheit pscheit force-pushed the test-replace-multiple-in-file branch from de02255 to 16b3a75 Compare March 6, 2015 11:47
@pscheit
Copy link
Contributor Author

pscheit commented Mar 6, 2015

Okay, moved the example to the php-file. Unfortunately I cannot re-generate the docs (all md files are emptied)

burzum pushed a commit to burzum/Robo that referenced this pull request Mar 15, 2015
@burzum
Copy link
Contributor

burzum commented Mar 15, 2015

@pscheit is there still any kind of issue with this? What exactly do you mean by

Unfortunately I cannot re-generate the docs

Is this broken for everyone now or just for you?

I referenced the wrong PR in my commit by the way. :(

@burzum
Copy link
Contributor

burzum commented Mar 15, 2015

According to @DavertMik it's OK to merge this, doing it now.

burzum added a commit that referenced this pull request Mar 15, 2015
Test Task ReplaceInFile for multiple strings in file
@burzum burzum merged commit a01a370 into consolidation:master Mar 15, 2015
@pscheit
Copy link
Contributor Author

pscheit commented Mar 16, 2015

you are right. Whats the best way to fix? seperate PR? Or should I commit into this branch?

I found the "bug" with the docs generation, my bad

pscheit added a commit to pscheit/Robo that referenced this pull request Mar 16, 2015
DavertMik added a commit that referenced this pull request Mar 23, 2015
fix PR #128 document replaceInFile with better example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants