gowrap fault injection #7056
Merged
gowrap fault injection #7056
+582
−845
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
stephanos
commented
Jan 7, 2025
| @printf $(COLOR) "Process go:generate directives..." | ||
| @go generate ./... | ||
| $(MAKE) copyright |
There was a problem hiding this comment.
Without this addition, the generated code has no copyright header. Fortunately, this step is idempotent (and fast).
There was a problem hiding this comment.
thoughts:
- should we copyright generated code?
- If we do - should we add it to the template? Otherwise it will still be "Uber 2020"
- I remember we have this discussion about "do we even need copyright at every file"? Where does it go?
There was a problem hiding this comment.
All good questions.
(1) The other generated code does, so I followed that here. (not a lawyer)
(2) Same as for (1)
(3) I'm told @alexshtin wants to follow up internally on that question.
There was a problem hiding this comment.
I am on it. Let this line be there for now.
stephanos
commented
Jan 7, 2025
| @@ -95,31 +94,3 @@ func inject0(f *fault, op func() error) error { | |||
| } | |||
| return f.err | |||
| } | |||
stephanos
force-pushed
the
gen-failinject
branch
6 times, most recently
from
67cbcdb to
ae4621b
Compare
stephanos
force-pushed
the
gen-failinject
branch
from
ae4621b to
3b1532f
Compare
ychebotarev
approved these changes
Jan 9, 2025
| @printf $(COLOR) "Process go:generate directives..." | ||
| @go generate ./... | ||
| $(MAKE) copyright |
There was a problem hiding this comment.
thoughts:
- should we copyright generated code?
- If we do - should we add it to the template? Otherwise it will still be "Uber 2020"
- I remember we have this discussion about "do we even need copyright at every file"? Where does it go?
alexshtin
approved these changes
Jan 14, 2025
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.
What changed?
Use gowrap for generating fault injection code. Similar to #7050.
Why?
Less code to maintain.
Before: 238 LOC.
After: 34 line Go Template.
How did you test it?
Existing tests + eyeballing the output.
Potential risks
Documentation
Is hotfix candidate?