Build test #251
Build test #251
Conversation
|
|
||
| - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 | ||
| - name: Setup Go | ||
| uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 | ||
| with: | ||
| go-version: 1.22 |
There was a problem hiding this comment.
Suggestion: You can change the name of the file to *-1.22.yaml
client.go
Outdated
| @@ -670,7 +670,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) { | |||
| var doErr, respErr, checkErr, prepareErr error | |||
|
|
|||
| for i := 0; ; i++ { | |||
| doErr, respErr, prepareErr = nil, nil, nil | |||
| //doErr, respErr, prepareErr = nil, nil, nil | |||
There was a problem hiding this comment.
Question: Why did we need to comment out this statement ?
There was a problem hiding this comment.
this was coming as an unused linter issue
There was a problem hiding this comment.
I commented out this statement as a part of unused linter issue.
Should I disable it like I did in previous prs?
| errors <- err | ||
| return | ||
| } | ||
| }() |
There was a problem hiding this comment.
Question: Was this added as a fix for lint check ?
Also, we should check if there were any errors added to errors and see if the test needs to fail
There was a problem hiding this comment.
Yes this was added as a part of lint fix.
There was a problem hiding this comment.
Can you check on the second part in the above comment ?
There was a problem hiding this comment.
go func() {
for err := range errors {
log.Println("Received error:", err)
}
}()
on adding this I saw that error. was being passed to the errors channel, and the test is designed to fail.
| errors <- err | ||
| return | ||
| } | ||
| }() |
There was a problem hiding this comment.
Can you check on the second part in the above comment ?
| - name: Setup Go | ||
| uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1 | ||
| with: | ||
| go-version: 1.22 |
There was a problem hiding this comment.
Suggestion: Both the workflows look the same to me, can you use matrix in workflow to make it a single file ?
I have added unit test coverage and linting to the ci.yml workflow. For more background, refer the https://hashicorp.atlassian.net/browse/IND-2338
The unit test coverage change was done to help understand the defects early in lifecycle and come up with a good solution for it. The linting was done to ensure that the codebase is consistent and maintainable and helps in improving the code quality and reducing errors.