chore: Update Golang version to 1.26.1 for v2.5.0 release#4602
chore: Update Golang version to 1.26.1 for v2.5.0 release#4602kislaykishore wants to merge 3 commits intov2.5.0_releasefrom
Conversation
kislaykishore
requested review from
vadlakondaswetha
and removed request for
a team
kislaykishore
added
execute-integration-tests
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the project's Golang dependency to version 1.26.1 in preparation for the v2.5.0 release. In addition to the version bump, the changes include necessary code adjustments to ensure compatibility with stricter Go vet checks, specifically addressing improper formatting arguments in logging and error reporting. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
kislaykishore
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Go version to 1.26.1 across the repository, including Dockerfiles, go.mod, and various setup scripts. It also refactors logging and error handling by removing redundant fmt.Sprintf calls within functions like fmt.Errorf and log.Printf. Feedback was provided to further optimize ExecuteGcloudCommandf calls by passing arguments directly to the function instead of pre-formatting the command string and using a "%s" format specifier.
| gcloudProvidePermissionCmd := fmt.Sprintf("alpha storage managed-folders set-iam-policy gs://%s/%s %s", bucket, managedFolderPath, localIAMPolicyFilePath) | ||
| _, err = operations.ExecuteGcloudCommandf(gcloudProvidePermissionCmd) | ||
| _, err = operations.ExecuteGcloudCommandf("%s", gcloudProvidePermissionCmd) |
There was a problem hiding this comment.
Instead of pre-formatting the command string with fmt.Sprintf and then passing it to ExecuteGcloudCommandf with a "%s" format, you can pass the arguments directly to ExecuteGcloudCommandf. This is cleaner and avoids redundant formatting.
| gcloudProvidePermissionCmd := fmt.Sprintf("alpha storage managed-folders set-iam-policy gs://%s/%s %s", bucket, managedFolderPath, localIAMPolicyFilePath) | |
| _, err = operations.ExecuteGcloudCommandf(gcloudProvidePermissionCmd) | |
| _, err = operations.ExecuteGcloudCommandf("%s", gcloudProvidePermissionCmd) | |
| _, err = operations.ExecuteGcloudCommandf("alpha storage managed-folders set-iam-policy gs://%s/%s %s", bucket, managedFolderPath, localIAMPolicyFilePath) |
References
- In integration tests, it is acceptable to use exec.Command to call external tools if the test's explicit purpose is to verify the system's behavior when interacting with that specific external tool.
| gcloudRevokePermissionCmd := fmt.Sprintf("alpha storage managed-folders remove-iam-policy-binding gs://%s/%s --member=%s --role=%s", bucket, managedFolderPath, serviceAccount, iamRole) | ||
|
|
||
| _, err := operations.ExecuteGcloudCommandf(gcloudRevokePermissionCmd) | ||
| _, err := operations.ExecuteGcloudCommandf("%s", gcloudRevokePermissionCmd) |
There was a problem hiding this comment.
The command can be passed directly to ExecuteGcloudCommandf without an intermediate fmt.Sprintf call.
| gcloudRevokePermissionCmd := fmt.Sprintf("alpha storage managed-folders remove-iam-policy-binding gs://%s/%s --member=%s --role=%s", bucket, managedFolderPath, serviceAccount, iamRole) | |
| _, err := operations.ExecuteGcloudCommandf(gcloudRevokePermissionCmd) | |
| _, err := operations.ExecuteGcloudCommandf("%s", gcloudRevokePermissionCmd) | |
| _, err := operations.ExecuteGcloudCommandf("alpha storage managed-folders remove-iam-policy-binding gs://%s/%s --member=%s --role=%s", bucket, managedFolderPath, serviceAccount, iamRole) |
References
- In integration tests, it is acceptable to use exec.Command to call external tools if the test's explicit purpose is to verify the system's behavior when interacting with that specific external tool.
| gcloudSecretAccessCmd := fmt.Sprintf("secrets versions access latest --secret %s", CredentialsSecretName) | ||
| creds, err := operations.ExecuteGcloudCommandf(gcloudSecretAccessCmd) | ||
| creds, err := operations.ExecuteGcloudCommandf("%s", gcloudSecretAccessCmd) |
There was a problem hiding this comment.
Simplify the command execution by passing the format string and arguments directly to ExecuteGcloudCommandf.
| gcloudSecretAccessCmd := fmt.Sprintf("secrets versions access latest --secret %s", CredentialsSecretName) | |
| creds, err := operations.ExecuteGcloudCommandf(gcloudSecretAccessCmd) | |
| creds, err := operations.ExecuteGcloudCommandf("%s", gcloudSecretAccessCmd) | |
| creds, err := operations.ExecuteGcloudCommandf("secrets versions access latest --secret %s", CredentialsSecretName) |
References
- In integration tests, it is acceptable to use exec.Command to call external tools if the test's explicit purpose is to verify the system's behavior when interacting with that specific external tool.
| cmd := fmt.Sprintf("storage buckets remove-iam-policy-binding gs://%s --member=serviceAccount:%s --role=roles/storage.%s", bucket, serviceAccount, permission) | ||
| _, err := operations.ExecuteGcloudCommandf(cmd) | ||
| _, err := operations.ExecuteGcloudCommandf("%s", cmd) |
There was a problem hiding this comment.
Avoid redundant formatting by passing the arguments directly to ExecuteGcloudCommandf.
| cmd := fmt.Sprintf("storage buckets remove-iam-policy-binding gs://%s --member=serviceAccount:%s --role=roles/storage.%s", bucket, serviceAccount, permission) | |
| _, err := operations.ExecuteGcloudCommandf(cmd) | |
| _, err := operations.ExecuteGcloudCommandf("%s", cmd) | |
| _, err := operations.ExecuteGcloudCommandf("storage buckets remove-iam-policy-binding gs://%s --member=serviceAccount:%s --role=roles/storage.%s", bucket, serviceAccount, permission) |
References
- In integration tests, it is acceptable to use exec.Command to call external tools if the test's explicit purpose is to verify the system's behavior when interacting with that specific external tool.
| gcloudDeleteManagedFolderCmd := fmt.Sprintf("alpha storage rm -r gs://%s/%s", bucket, managedFolderPath) | ||
|
|
||
| _, err := ExecuteGcloudCommandf(gcloudDeleteManagedFolderCmd) | ||
| _, err := ExecuteGcloudCommandf("%s", gcloudDeleteManagedFolderCmd) |
There was a problem hiding this comment.
Simplify by passing the format string and arguments directly to ExecuteGcloudCommandf.
| gcloudDeleteManagedFolderCmd := fmt.Sprintf("alpha storage rm -r gs://%s/%s", bucket, managedFolderPath) | |
| _, err := ExecuteGcloudCommandf(gcloudDeleteManagedFolderCmd) | |
| _, err := ExecuteGcloudCommandf("%s", gcloudDeleteManagedFolderCmd) | |
| _, err := ExecuteGcloudCommandf("alpha storage rm -r gs://%s/%s", bucket, managedFolderPath) |
References
- In integration tests, it is acceptable to use exec.Command to call external tools if the test's explicit purpose is to verify the system's behavior when interacting with that specific external tool.
| gcloudCreateManagedFolderCmd := fmt.Sprintf("alpha storage managed-folders create gs://%s/%s", bucket, managedFolderPath) | ||
|
|
||
| _, err := ExecuteGcloudCommandf(gcloudCreateManagedFolderCmd) | ||
| _, err := ExecuteGcloudCommandf("%s", gcloudCreateManagedFolderCmd) |
There was a problem hiding this comment.
Simplify by passing the format string and arguments directly to ExecuteGcloudCommandf.
| gcloudCreateManagedFolderCmd := fmt.Sprintf("alpha storage managed-folders create gs://%s/%s", bucket, managedFolderPath) | |
| _, err := ExecuteGcloudCommandf(gcloudCreateManagedFolderCmd) | |
| _, err := ExecuteGcloudCommandf("%s", gcloudCreateManagedFolderCmd) | |
| _, err := ExecuteGcloudCommandf("alpha storage managed-folders create gs://%s/%s", bucket, managedFolderPath) |
References
- In integration tests, it is acceptable to use exec.Command to call external tools if the test's explicit purpose is to verify the system's behavior when interacting with that specific external tool.
| gcloudCopyFileCmd := fmt.Sprintf("alpha storage cp %s gs://%s/%s/", srcfilePath, bucket, destFilePath) | ||
|
|
||
| _, err := ExecuteGcloudCommandf(gcloudCopyFileCmd) | ||
| _, err := ExecuteGcloudCommandf("%s", gcloudCopyFileCmd) |
There was a problem hiding this comment.
Simplify by passing the format string and arguments directly to ExecuteGcloudCommandf.
| gcloudCopyFileCmd := fmt.Sprintf("alpha storage cp %s gs://%s/%s/", srcfilePath, bucket, destFilePath) | |
| _, err := ExecuteGcloudCommandf(gcloudCopyFileCmd) | |
| _, err := ExecuteGcloudCommandf("%s", gcloudCopyFileCmd) | |
| _, err := ExecuteGcloudCommandf("alpha storage cp %s gs://%s/%s/", srcfilePath, bucket, destFilePath) |
References
- In integration tests, it is acceptable to use exec.Command to call external tools if the test's explicit purpose is to verify the system's behavior when interacting with that specific external tool.
1 participant
Upgrade the Golang version to 1.26.1. Includes fixes for go vet compatibility.