Don't read stdout into memory when not needed #143
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
soenkehahn
force-pushed
the
optimization
branch
from
37e54b8 to
c96c9aa
Compare
soenkehahn
force-pushed
the
optimization
branch
from
c96c9aa to
e8cbb49
Compare
soenkehahn
changed the title
soenkehahn
commented
Aug 6, 2021
soenkehahn
commented
Aug 6, 2021
casey
reviewed
Aug 8, 2021
Co-authored-by: Casey Rodarmor <[email protected]>
Co-authored-by: Casey Rodarmor <[email protected]>
Co-authored-by: Casey Rodarmor <[email protected]>
Co-authored-by: Casey Rodarmor <[email protected]>
Co-authored-by: Casey Rodarmor <[email protected]>
casey
reviewed
Aug 9, 2021
| @@ -73,6 +73,7 @@ jobs: | |||
| - name: Install just | |||
There was a problem hiding this comment.
If you want to get a specific version of just, you can do curl --proto '=https' --tlsv1.2 -sSf https://just.systems/install.sh | bash -s -- --to DEST --tag VERSION
There was a problem hiding this comment.
Does that work on windows?
There was a problem hiding this comment.
Uhhhhhhh, I don't actually know 😅
There was a problem hiding this comment.
Yeah, no, it doesn't. :|
There was a problem hiding this comment.
I've opened this issue to explain what the actual problems are: casey/just#943. I.e. I'm not at all interested in getting a specific just version, the newest would be just fine. I just want a way that works on github CI on all platforms.
soenkehahn
force-pushed
the
optimization
branch
from
42f02c9 to
478c938
Compare
This was referenced Aug 9, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cradleneeds to read what's written tostdoutinto memory when the user wants to capturestdout, e.g. withStdoutUntrimmed. However currently it also reads all ofstdoutinto memory when not capturingstdout, which is unnecessary. It's also conceivable (when there's lots of output) that this may cause problems. This PR avoids collectingstdoutinto memory when it's not captured.The same could be done for
stderr. Not sure whether that's as important as doing it forstdout. In any case it should be done in a separate PR.This PR is half a fix for #26.