step depends_on ignore empty strings #3008
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3008 +/- ##
==========================================
+ Coverage 34.81% 34.83% +0.02%
==========================================
Files 228 228
Lines 14751 14757 +6
==========================================
+ Hits 5135 5141 +6
Misses 9238 9238
Partials 378 378 ☔ View full report in Codecov by Sentry. |
| position: 0, | ||
| name: "echo env", | ||
| group: "", | ||
| dependsOn: []string{""}, |
There was a problem hiding this comment.
@6543 Why do you want to support such a wrong configuration? Shouldn't we treat it like an error if someone writes: depends_on: [""] instead of depends_on: [] Or is it because of this super weirdish way go-yaml is converting the slice?
There was a problem hiding this comment.
I just want to let the server follow our idea: if run and on strange edgecases make sure it still works
the linter of course could point out a warning, but if you do depends_on: [""] as a naive user i expect it to not depend on something ... since the string is empty right 😆
There was a problem hiding this comment.
I would just throw an error instead of supporting such configs
There was a problem hiding this comment.
Well I'm for supporting users
There was a problem hiding this comment.
Yes, but not bad habits and wrong configs. Using an empty string is just wrong IMO.
There was a problem hiding this comment.
well we now even have a good use-case, and a user intuitively try to use it that way already
There was a problem hiding this comment.
I still think it supports wrong configs. The mentioned issue was a bug solved by #3097
6543
added
enhancement
|
#3097 got merged |
Just extend the test and ignore empty string dependencies
close #3055
close #3047