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.
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
feat: add parallel cache support #7131
feat: add parallel cache support #7131
Changes from 5 commits
c6cadbc
1ba6e58
10f2de7
5af2c0a
d3cb705
ea7219e
e06771d
2aef0f8
8cecc03
30dec99
10560c6
29530e0
6e52cd4
ac074e1
3c8e02c
f0dbe27
c0d1120
d25c533
5b227da
860c4c0
c9ee48a
efef110
5f96e9d
b22d37d
98a2c72
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO it shouldn't be exported, but defined only for this particular run, that is supported by external parallelisation. Exporting will keep that value and will use that feature also for regular run (without
xargs
) and I am not sure if it should be like this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your proposal is returning
some playground
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not thought about pipes and variable visibility. It should be something like
FOO=bar bash -c 'somecommand someargs | somecommand2'
(source), in this case variable is available on each level of the pipeline. It makes it a little bit more complicated, but enables feature for single run instead of doing it globally. I won't fight for it, just a suggestion 🙂.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it makes it overcomplex and I'm ok to have env exported. I assume that one either will want it each time, or never - and if they need sometimes, they will figure out the way.
bash is also less portable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solved in #7131 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idea for future (out of this PR scope) - we can detect that file was modified since it was read initially, and do the backilling based on that indicator, without env var.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implemented in 2aef0f8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder about putting
@
to silence warning and check!== false
for every method here... any better suggestion?