search

bitrise-io / bitrise

Bitrise runner CLI - run your automations on your Mac or Linux machine -
https://www.bitrise.io/cli
MIT License
840 stars 131 forks source link

CLI should throw an error or warning on duplicate workflow names #517

Open koral-- opened 7 years ago

koral-- commented 7 years ago

Inspired by https://github.com/bitrise-io/bitrise-workflow-editor/issues/172 and https://github.com/bitrise-io/bitrise-workflow-editor/issues/173#issuecomment-311123000.

Currently, CLI allows duplicated names but "sees" only last one (it is listed by bitrise workflows and run by bitrise run).

Triggers validation is already there (e.g. error is thrown immediately if non-existent workflow is triggered), so it seems that workflow names can also be validated.

viktorbenei commented 7 years ago

This is unfortunately due to how the YAML lib works. We don't get any errors or indication, only a hash, and as a hash can contain a key at a level only once one of those workflows are just lost before the CLI gets the data (hash).

I think a regex based workaround could be applied, but I don't think we'll have the time in the near future. If anyone wants to send a PR we'd be glad to review it!

koral-- commented 7 years ago

OK, indeed it seems that YAML parser should be responsible for validating this. There are even (at least) 2 open PRs for that: https://github.com/go-yaml/yaml/pull/248 and https://github.com/go-yaml/yaml/pull/186 Moreover according to this issue: https://github.com/go-yaml/yaml/issues/154 current behavior is invalid and first (not last) mapping should be chosen.

viktorbenei commented 7 years ago

current behavior is invalid and first (not last) mapping should be chosen.

to be honest I don't think it should drop either, or at least it should definitely return an error so that the caller side (e.g. our CLI) can decide whether it's a critical issue (we definitely would consider this an error / validation fail)