-
Notifications
You must be signed in to change notification settings - Fork 623
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: attempt to fix panics for unresolvable imports by using GogoResolver (backport #7277) #7282
Conversation
Cherry-pick of 79ddda5 has failed:
To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Test failure.. 👀 |
I cannot reproduce this unit test failure locally. All the tests pass on my machine |
@@ -21,7 +21,7 @@ jobs: | |||
steps: | |||
- uses: actions/setup-go@v3 | |||
with: | |||
go-version: 1.19 | |||
go-version: 1.22 |
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.
ah yea, this little trick 😅 did you ever get to the bottom of why tf this works?
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.
I did not! 🫠
I think my recommendation would be to close this PR and not backport the change to the v7.8.x release line due to the instability and unpredictability with populating the module query safe allow list on different versions of Go. This backport PR demonstrates that if our tests are run on go1.19 and go1.22 then the allowlist will yield different results. I think we should consider different approaches for populating the allow list for module safe queries in future. Maintaining the allow list manually is not optimal and poor ux, but there seems to be instability in the older versions of this proto/gogoproto code. What do others think? |
Description
Uses the cosmos/gogoproto registry as opposed to including the entire merged global files registry.
closes: #7259
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.This is an automatic backport of pull request #7277 done by [Mergify](https://mergify.com).