Don't wait on stdin by default for batch commands#680
Conversation
|
Many thanks for submitting this thoughtful PR, and investing all that time. It makes a lot of sense, we will discuss it as a team, and post next steps here. |
|
Yes, I also wanted to thank you, @HenrikPoulsen, for your thoughtful PR, and to apologize for the headache this has caused you! We will get on this, and I hope you're now able to explore Batch Changes. Thanks! |
|
Hi @HenrikPoulsen 👋 Thank you for this thorough proposal for making Batch Changes more intuitive to use. We decided to accept this proposal and change the behavior and not wait for stdin to provide input. :) We will likely do the following:
We will prioritize this in our next sprint planning on Tuesday. |
|
Thanks for your PR, Henrik! We ended up doing something similar, but slightly different — we've altered This will be coming in the next Thanks again! |
|
Sounds great. Thanks! |
So I've been on a several month long goose chase.
I have been unable to create sourcegraph batches due to it hanging when running things like
src batch previewin the folder that has a batch spec file.It would hang forever on the message
⠋ Parsing batch specMaybe the issue is already obvious here, but I was in contact with Sourcegraph staff about this and did show them my invocation, and they also asked me to try some invocations with the same issue.
I tried this on windows, on wsl2, and even created a virtual box ubuntu vm, all had the same issue. Then finally I dusted off an old mac pro and tried it there, the same issue. So it obviously wasn't a hardware problem.
At that point I realized the cli is open source so I just tried to reproduce it with a debugger attached, and it was easily reproducible.
The problem is that it seems that
srcrequires you to specify-fwhen you want to use a file, and it does not by default find thebatch.yamlin the current folder.Instead it defaults to reading from stdin and expects the batch file from there. This is not printed however so an end user is completely stumped as to what is going on.
The help text for the commands does not mention this either, so for the casual user it would not be obvious that you cannot ommit
-fand instead are expected to add it or do something likecat batch.yaml | src batch preview.This PR serves as one example how to solve this.
I set the default value to if
-fis missing to bebatch.yaml, which is whatsrc batch newcreates by default.So to me it seems obvious that by default the batch commands looks for the file that is created when a new batch is created with the default name.
This is obviously a breaking change, but I cannot speak to how much of an issue this is. If you know that a lot of tooling exists that rely on this, then you likely would not want this approach.
Another approach would be to simply print "waiting for stdin" when stdin has been indicated, so the user is actually made aware that something is wrong here.
Personally I prefer to default to looking for
batch.yamlfor the reason mentioned above. But it's your tool so please do advise here. Thanks