Conversation
|
Surge PR preview deployment succeeded. View it at https://woodpecker-ci-woodpecker-pr-5988.surge.sh |
|
this pr also to be used in https://codeberg.org/lafriks/woodpecker-pipeline-transform/pulls/33#issuecomment-9962346 |
|
I don't like this change. It would be nice if steps and services have a consistent syntax. I personally think we should just forbid duplicated names (e.g. via the linter), for both steps and services. |
This comment was marked as resolved.
This comment was marked as resolved.
I agree, the linter should check for duplicate names, I also prefer list not a map syntax |
|
I removed the warning and added an check if it is a list and was declared twice |
|
also we support duplicated step names and it has no conflict so why should we now block it? |
This comment was marked as resolved.
This comment was marked as resolved.
|
Because it's not working properly: #4385 (Didn't try myself, but since that issue was opened nothing was changed to fix it) |
|
might have missed that issue but it should be solved since worker api for steps changed to uuid for identifyers |
|
I think this issue was opened after we changed that. But I would also have to try that first. |
|
I just tested it again, and the issue is the The test-1 pipeline has a |
|
well depends_on is a fundamental feature ... forgot about that conflict ... so in this case as soon as it is used we need to lint for that. i have a look what is more complex (denai of it allways or just at the dag case) |
| // ContainerMap contains collection of containers. | ||
| type ContainerMap struct { | ||
| ContainerMap map[string]*Container | ||
| Duplicated []string |
There was a problem hiding this comment.
Do we have to put this into the parsing code? I think the linter should check separately for duplicated entries and for parsing just use the ContainerList
as services must have unique names as they are used for there host names, and services start all in parallel anyway, we should prefer map syntax.
because the map syntax in yaml represents how they work best:
woodpecker/.woodpecker/securityscan.yaml
Lines 34 to 41 in 4e0b91f
woodpecker/.woodpecker/test.yaml
Lines 144 to 159 in 4e0b91f
this change also makes sure re-serialize works just fine :)