Changes made to connectivity/check/logging.go #2210
Conversation
|
Commit b71a58d does not match "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
|
Thanks for the PR. It doesn't build, could you fix, build and test it and re-push a fresh version? |
|
@joestringer I have some questions which might not be fully within the scope of this issue, so can I message you on slack ? |
|
I would suggest posting in #development channel on Slack. I am not deeply involved with cilium-cli, I just noticed the issue. Posting in that public channel should provide opportunities for other developers in the community to see your messages and provide input. |
|
Commits b71a58d, 05deb85 do not match "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
There was a problem hiding this comment.
Thanks for the PR. A comment below to address.
Additionally, as mentioned in the issue, please link the issue in the description of the commit msg. You can also squash the commits into one as there is no need for multiple commits here. For more info on how to format commit msgs, please see https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#submit-pr.
|
@christarazi Thanks for the comment! I'm quite new to open source, but I'm slowly but surely getting the hang of it. |
connectivity/check/logging.go
Outdated
| @@ -312,7 +312,8 @@ func (t *Test) failCommon() { | |||
| // will go directly to the user-specified writer. | |||
| func (t *Test) Fail(a ...interface{}) { | |||
| t.log(fail, a...) | |||
| t.failCommon() | |||
| t.failCommon(t.ctx.Context()) | |||
|
|
|||
There was a problem hiding this comment.
Nit: extra line here that's not necessary
Before: Each test caller set up signal handling indiviually After: A globally declared SIGTERM handler now manages signals Fixes: cilium#2042 Signed-off-by: Archit H Barve <[email protected]>
|
@christarazi I have reworded the commit message and removed the extra line at line 316. |
|
@AHB102 Seems like the code has some compilation errors. |
Signed-off-by: Archit H Barve <[email protected]>
Hi there @joestringer, I forgot to add a PR, resolved issue #2042, added context as a parameter to the failCommon method, to allow each test caller to pass their own context down when invoking Fail(), Failf(), Fatal(), and Fatalf()