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

Implemented dry run mode #126

Merged
merged 3 commits into from
Aug 5, 2018
Merged

Implemented dry run mode #126

merged 3 commits into from
Aug 5, 2018

Conversation

bebbs
Copy link
Contributor

@bebbs bebbs commented Jul 28, 2018

Added a --dry-run flag that compiles and steps through each task, but does not execute them. The commands that would have been run are printed. See #125.

Added a --dry-run flag that compiles and steps through each task, but does not execute them. The commands that would have been run are printed. See go-task#125.
Copy link
Member

@andreynering andreynering left a comment

Choose a reason for hiding this comment

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

Hey @bebbs, thank for the pull request!

What about calling it --dry instead of --dry-run?

A simple test for it would be nice, but if you don't want to write it, I can do that after merging this PR


Note to myself: in the future we should disallow some flags to be used together, like --silent and --dry or --silent and --verbose (not in this PR)

README.md Outdated
## Dry Run Mode

Dry run mode (`--dry-run`) compiles and steps through each task, printing the commands
that would be run without executing them. This is useful for debugging your Taskfiles.
Copy link
Member

Choose a reason for hiding this comment

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

👍 for adding documentation for it

@@ -12,6 +12,7 @@ function __list() {

_arguments \
'(-d --dir)'{-d,--dir}': :_files' \
'(--dry-run)'--dry-run \
Copy link
Member

Choose a reason for hiding this comment

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

👍 for remembering to update this file

@bebbs
Copy link
Contributor Author

bebbs commented Jul 29, 2018

No problem!

What about calling it --dry instead of --dry-run?

Done.

A simple test for it would be nice, but if you don't want to write it, I can do that after merging this PR

I couldn't see where to add tests for the effects of flags, apologies if I've missed it. Agree it would be nice to have them, happy for you to merge and add the test if you'd prefer, or let me know where you would like to see one and I'll add it to this PR?

@andreynering
Copy link
Member

Indeed there's no test for flags currently, but we can have a test on task_test.go that test the behavior with the DryRun prop set on an Executor

Optional in your part, but you can do if you're feeling adventurous 🙂

@bebbs
Copy link
Contributor Author

bebbs commented Jul 31, 2018

All done @andreynering, I've added a test case that would touch a file, checks that the task is compiled without errors, and checks that the file does not exist after running.

@andreynering
Copy link
Member

@bebbs Great! Thanks again for the contribution

I'll do very small changes after merging

@andreynering andreynering merged commit 27fc4c4 into go-task:master Aug 5, 2018
@andreynering andreynering mentioned this pull request Aug 5, 2018
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.

2 participants