Fix deluge of linter complaints on branch merge#646
Conversation
Shaptic
changed the title
| // we had one or more missing options, combine these all into a single error. | ||
| errString := "The following required configuration parameters are missing:" | ||
| var errBuilder strings.Builder | ||
| errBuilder.WriteString("The following required configuration parameters are missing:") |
There was a problem hiding this comment.
Linter caught these string building thingies?
:respect:
There was a problem hiding this comment.
"caught" is a stretch for short strings, but yes 😆
There was a problem hiding this comment.
We have a ton of linting failures on the codebase in theory, but the CI only runs on touched files so we can update them piecemeal. This is a common one
| if batchEnd > len(insertableEvents) { | ||
| batchEnd = len(insertableEvents) | ||
| } | ||
| batchEnd := min(batchStart+maxEventsPerBatch, len(insertableEvents)) |
There was a problem hiding this comment.
Linter caught this too?
There was a problem hiding this comment.
This is all from piping the golangci-lint job logs to gpt via copilot, and this complaint specifically came from the modernize linter, yes.
| os.Exit(1) | ||
| } | ||
| fmt.Println(string(out)) | ||
| _, _ = fmt.Fprintln(os.Stdout, string(out)) |
There was a problem hiding this comment.
Is this needed?
There is no requirement that you are supposed to take the return values and do anything with it.
This just looks weird.
If the linter really is complaining about these kinda things then perhaps we need to do update linter settings somewhere?
There was a problem hiding this comment.
That's from https://github.com/kisielk/errcheck, i.e. error if errors aren't check. I agree that it's a little aggressive, lemme see if there are settings
There was a problem hiding this comment.
We can add it to the ignorelist for the checker, but I think a one-off is better here. We may want to ensure writes match buffers in other cases 🤷
* Bump soroban-env to next protocol version (#595) - Add required config key to mocked ledger entries - Integrate unstable-next-api into curr (dropped arg) * Update XDR and Core runs to test P26 integration (#629) - Bump Golang to 1.25 and linter to be compatible - Bumped Go SDK to latest @protocol-next - Added P26 limits from quickstart and removed P24 limits * Fix deluge of linter complaints on branch merge (#646) --------- Co-authored-by: Siddharth Suresh <siddharth@stellar.org>
* Bump soroban-env to next protocol version (#595) * Bump soroban-env to next protocol version * Add required config key to mocked ledger entries * Integrate unstable-next-api into curr (dropped arg) * Bump soroban-env SHA to latest (#611) * Update XDR and Core runs to test P26 integration (#629) * Split the single core_version input into core_deb_version and core_docker_img to accommodate differences. * Bump Core versions for P25 and add P26 run * Bump Golang to 1.25 and linter to be compatible * Bumped soroban-env-host curr to latest commit SHA in repo * Bumped Go SDK to latest @protocol-next * Added P26 limits from quickstart and removed P24 limits * Bump crate dependencies to latest Protocol 26 versions (#640) * Remove unimplemented GitHub Action (#643) Snuck in with a `git add -A` on accident * Bump Core to Protocol 26 release candidate (#642) * Bump Core to p26 release candidate * Bump Go SDK version to latest * Bump to official patched env * Ensure we strip NUL bytes before `CString::new` (#632) * Fix deluge of linter complaints on branch merge (#646) * Release v26.0.0 (#647) * Tag off changelog for major release * Bump Go SDK to official P26 version * Bump CI to latest stable core for a final run * Add GHA to automatically update soroban-env on protocol-next (#645) * Preserve version string in Cargo.toml on automatic dependency bump (#672) * Preserve version= in Cargo.toml on bump * Simplify code: use a function, better names * Bump to soroban-env to latest version (#673) --------- Co-authored-by: George <Shaptic@users.noreply.github.com> Co-authored-by: Siddharth Suresh <siddharth@stellar.org> Co-authored-by: George <george@stellar.org>
2 participants
What
Appease CI.
Why
The branch-merge in #623 is complaining with 6 or 7 thousand linter complaints so I asked copilot to fix them.
Known limitations
n/a