-
Notifications
You must be signed in to change notification settings - Fork 3.8k
docs: fix function names in fuzzing test documentation #8044
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
Conversation
|
Hi @fish98. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signed-off-by: Jiongchi Yu <[email protected]>
| // FuzzNoTearDownWithDownload() implements a fuzzer | ||
| // similar to FuzzCreateContainerNoTearDown() and | ||
| // FuzzCreateContainerWithTearDown(), but it takes a |
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.
It looks like I've missed some history here, since this isn't the only one where it isn't matching, but the function name is FuzzIntegNoTearDownWithDownload, not FuzzNoTearDownWithDownload.
For this to fit conventional Go style, it'd also need to change like this:
| // FuzzNoTearDownWithDownload() implements a fuzzer | |
| // similar to FuzzCreateContainerNoTearDown() and | |
| // FuzzCreateContainerWithTearDown(), but it takes a | |
| // FuzzIntegNoTearDownWithDownload implements a fuzzer | |
| // similar to FuzzIntegCreateContainerNoTearDown and | |
| // FuzzIntegCreateContainerWithTearDown, but it takes a |
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.
Thanks for your review. I have rechecked the whole file following your suggestion, and updated the rest of the function names :)
Signed-off-by: Jiongchi Yu <[email protected]>
estesp
left a 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.
LGTM
|
/retest |
|
@fish98: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/ok-to-test |
Update fork-external/main with upstream main at commit [081d818](containerd@081d818) Marged upstream container/main into fork-external/main Related work items: containerd#7864, containerd#7954, containerd#8041, containerd#8044, containerd#8051, containerd#8062, containerd#8096, containerd#8103, containerd#8109, containerd#8110, containerd#8113, containerd#8114, containerd#8119, containerd#8120, containerd#8128, containerd#8130, containerd#8134, containerd#8140, containerd#8142, containerd#8143, containerd#8152, containerd#8154, containerd#8162, containerd#8164, containerd#8165, containerd#8172, containerd#8173, containerd#8177, containerd#8178, containerd#8181, containerd#8183, containerd#8187, containerd#8188, containerd#8189, containerd#8190, containerd#8191, containerd#8192, containerd#8193
The introduction of FuzzIntegNoTearDownWithDownload is confusing with the duplicate names in container fuzzer.
Signed off by: Jiongchi Yu [email protected]