Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For internal use only: add a -text-only flag to src batch [apply|preview] #562

Merged
merged 5 commits into from
Jul 8, 2021

Conversation

mrnugget
Copy link
Contributor

@mrnugget mrnugget commented Jul 2, 2021

This adds a -text-only flag to src batch [apply|preview] that is for internal use only.

It's a highly-experimental feature that should not be used if you're not sure about what you're doing.

Copy link
Contributor

@LawnGnome LawnGnome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One semi-related thought: right now we have a fairly half-arsed attempt at a non-fancy mode in output to handle the case where the output isn't going directly to a terminal. I'm thinking we might want to remove that and use this as the default once we have this fully implemented.

@mrnugget
Copy link
Contributor Author

mrnugget commented Jul 6, 2021

One semi-related thought: right now we have a fairly half-arsed attempt at a non-fancy mode in output to handle the case where the output isn't going directly to a terminal. I'm thinking we might want to remove that and use this as the default once we have this fully implemented.

Ah, yes. I forgot about that. Question is though: is the text output for human or machine consumption? Because what's in here is definitely for machines.

@LawnGnome
Copy link
Contributor

Question is though: is the text output for human or machine consumption?

More human, but I'm not super convinced it's actually very useful for that.

@mrnugget mrnugget requested a review from a team July 8, 2021 11:22
@mrnugget mrnugget marked this pull request as ready for review July 8, 2021 11:22
@mrnugget
Copy link
Contributor Author

mrnugget commented Jul 8, 2021

@sourcegraph/batchers As discussed yesterday: I want to get this in. I have some ideas on how to reduce the code duplication, but since that would require touching the old code I'd rather add a bit of duplication for now to get something running fast than to touch and possibly break the old code. For that I want to take my time.

@mrnugget mrnugget changed the title Start hacking on text-only output For internal use only: add a -text-only flag to src batch [apply|preview] Jul 8, 2021
@mrnugget mrnugget merged commit bc8ab68 into main Jul 8, 2021
@mrnugget mrnugget deleted the mrn/text-only branch July 8, 2021 11:27
scjohns pushed a commit that referenced this pull request Apr 24, 2023
…view]` (#562)

* Start hacking on text-only output

* Get stupidest text only output working

* Print JSON objects per line when logging text

* Remove last use of output in text-only mode

* Update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants