feat(file) allow condition to only check for file existence #382
Conversation
dduportal
added
condition
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
| if passing { | ||
| logrus.Infof("%s Condition on file %q passed", result.SUCCESS, f.spec.File) | ||
| } else { | ||
| logrus.Infof("%s Condition on file %q did not pass", result.FAILURE, f.spec.File) |
There was a problem hiding this comment.
What would you think to use a different message than failiure?
| logrus.Infof("%s Condition on file %q did not pass", result.FAILURE, f.spec.File) | |
| logrus.Infof("%s Condition on file %q did not pass", result.ATTENTION, f.spec.File) |
"\u26A0"
There was a problem hiding this comment.
It feels weird to use « attention » since it is the symbol used when a target changes a file. Might be something to reconsider overall: choosing 2 symbols for failure and not passing, and 2 other symbols for « success » and « changed »
There was a problem hiding this comment.
But the fact of sharing the same error symbol is why I took care of having 2 really different text messages to easily discern failure from condition not passing. Maybe we could remove the « cross » from failure since it generate a clear error message in red.
There was a problem hiding this comment.
Excepted minor comment, this pull request looks great to me. Thanks @dduportal for the improvement
|
Feel free to merge once you are ready |
Co-authored-by: Olivier Vernin <me@olblak.com>
Fix #290
No need for a specific atttribute: only have to disable the source input in condition.
2 examples added in the PR to show how to use it + test units to cover additional cases.
It also add improved output for end user:
Test
To test this pull request, you can run the following commands:
Additional Information
Tradeoff
Potential improvement