-
Notifications
You must be signed in to change notification settings - Fork 511
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
fix: Remove always nil return value #778
Conversation
@@ -175,7 +175,6 @@ func (p *Parser) interpret(cfg *BaseConfig) hcl.Diagnostics { | |||
} | |||
} | |||
} | |||
return nil |
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.
Return value is always nil
and will actually cause a panic in callers I believe
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.
hcl.Diagnostics is a slice:
type Diagnostics []*Diagnostic
nil slice is valid and diags.HasErrors
won't cause panic.
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.
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.
Cool, thanks for the context. Should we still remove the return value? It's not used
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.
Sure.
* upstream/main: (22 commits) fix: Skip reattached provider update checks (cloudquery#801) fix: Add Git Binary To Docker chore(main): Release v0.24.1 (cloudquery#780) fix: Test flakiness (cloudquery#790) feat: Store Policy Output (cloudquery#709) fix: Panic in loadPolicyFromSource (cloudquery#787) fix: Panic in OsFs.downloadFile (cloudquery#789) fix: Error classifier improvements (cloudquery#788) fix: Console log log level (cloudquery#786) refactor: Use pointer for connection receiver (cloudquery#782) fix(deps): Update module github.com/cloudquery/cq-provider-sdk to v0.10.2 (cloudquery#784) refactor: Use copy instead of loop (cloudquery#781) fix: Remove always nil return value (cloudquery#778) test(fetch): Fix assertions to match new test provider (cloudquery#777) test(fetch): Pin test provider version (cloudquery#776) chore(main): Release v0.24.0 (cloudquery#749) fix: Upgrade protocol version to V5 (cloudquery#774) fix: Check policy version on it's core version (cloudquery#773) fix: Policy output file name (cloudquery#770) fix: Policy executor (cloudquery#769) ...
🎉 Thank you for making CloudQuery awesome by submitting a PR 🎉
Summary
New linter caught this. I'm doing this in a separate PR to make verifying the change easier
Use the following steps to ensure your PR is ready to be reviewed
go fmt
to format your code 🖊golangci-lint run --new-from-rev main
🚨 (install golangci-lint here)