-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
chore: copyloopvar #20371
chore: copyloopvar #20371
Conversation
Important Auto Review SkippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 99 files out of 156 files are above the max files limit of 50. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
This seems to get around the issue with gomod2nix by referring tothe latest commit when we build it. ...nope |
@@ -19,7 +19,7 @@ then | |||
if ! command -v gomod2nix &> /dev/null | |||
then | |||
echo "gomod2nix could not be found in PATH, installing..." | |||
go install github.com/nix-community/gomod2nix@latest | |||
go install github.com/nix-community/gomod2nix@872b63ddd28f318489c929d25f1f0a3c6039c971 |
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.
this script need to change a little bit:
if command -v nix &> /dev/null
then
nix develop .. -c gomod2nix
else
if ! command -v gomod2nix &> /dev/null
then
# install gomod2nix manually
echo "gomod2nix could not be found in PATH, installing..."
go install github.com/nix-community/gomod2nix@master
fi
gomod2nix
fi
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.
the nix develop
version should be more stable, because it also setup correct go version according to configs in go.mod
.
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.
Awesome, thank you! I'll get your changes in in a little bit :)
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.
Please revert all go.mod changes we are a library so bumping this number that often should be done
It's a "both" situation. copyloopvar requires go > 1.22 in go.mod or its magic can't work |
Please revert the go mod changes. We won't accept the pr as it currently stands |
Can you make the lint green @faddat |
Right, as that behavior only happens from 1.22: https://go.dev/blog/loopvar-preview |
closing due to staleness |
Description
Adds the copyloopvar linter, and uses it.
Also adds a few newer linters, that didn't make any findings but would be good to have:
Also updates go in CI to v1.22.3 everywhere, and sets each go.mod's minimum go to 1.22.2
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...