Conversation
| Vendor string `help:"CI/CD vendor (auto-detected if the file name matches vendor path and name - otherwise, needs to be specified)" short:"v"` | ||
| Output string `help:"Custom path to save the converted pipeline (default: .buildkite/pipeline.<vendor>.yml)" short:"o"` | ||
| Timeout int `help:"Timeout in seconds for conversion" default:"300"` | ||
| Timeout int `help:"Timeout in seconds for conversion (increase for large pipelines, decrease to fail conversion on timeout)" default:"300"` |
There was a problem hiding this comment.
I'm not sure if it's clear to me what's the intent of the current wording, especially around decreasing the timeout value.
There was a problem hiding this comment.
@omehegan we might need your assistance here as I tried to convey your explanation to the best of my ability, and it didn't seem to have worked.
There was a problem hiding this comment.
Yeah, I don't know that this needs a change, maybe I'm biased here as I know what timeout is, but I think it's generally accepted as a term and function?
decrease to fail conversion on timeout, what happens if I set to 0?
There was a problem hiding this comment.
@mcncl if the timeout is set to zero, same thing happens as when the timeout is shorter than the conversion needs to run - "panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x10 pc=0x104d337d0]"
There was a problem hiding this comment.
Should we add a warning to advise users against setting the timeout to zero, to be on the safe side?
There was a problem hiding this comment.
No, that should be handled in the code
Description
Changes
This PR changes nothing in the way CLI fuctions, but changes help descriptions for auto-detection on vendors and the use of timeouts.
-->
Testing
Does not apply.
Disclosures / Credits
Minor fix in the descriptions.
-->