Skip to content

chore: upgrade Go to 1.26#676

Open
sylr wants to merge 4 commits intomainfrom
chore/upgrade-go
Open

chore: upgrade Go to 1.26#676
sylr wants to merge 4 commits intomainfrom
chore/upgrade-go

Conversation

@sylr
Copy link
Copy Markdown

@sylr sylr commented Mar 11, 2026

Summary

  • Upgrade Go toolchain from 1.24 to 1.26 in nix dev shell
  • Modernize flake.nix to use stable/unstable nixpkgs split
  • Use allowUnfreePredicate instead of allowUnfree = true
  • Update nix flake lock file

🤖 Generated with Claude Code

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 996923c6-f172-41c2-a187-d809c66e8e40

📥 Commits

Reviewing files that changed from the base of the PR and between 0591604 and 71bbcd8.

📒 Files selected for processing (1)
  • dev.Dockerfile
✅ Files skipped from review due to trivial changes (1)
  • dev.Dockerfile

Walkthrough

This PR upgrades the development environment to Go 1.26 and refactors the Nix flake to use separate stable/unstable nixpkgs inputs and a direct go_1_26 selection. It also tweaks pagination field access in Stripe/CurrencyCloud clients, adjusts duration/timestamp serialization in migrations/storage, and updates related tests and the dev Dockerfile.

Changes

Cohort / File(s) Summary
Nix flake / Dev environment
flake.nix, dev.Dockerfile
Bumped description and Go toolchain to 1.26; removed goVersion/overlay indirection and selected go_1_26 directly; added nixpkgs-unstable input and dual-pkgs import per system; restricted unfree packages via config.allowUnfreePredicate; updated dev container Go image and tool pins.
Stripe pagination tweaks
internal/connectors/plugins/public/stripe/client/accounts.go, internal/connectors/plugins/public/stripe/client/external_accounts.go, internal/connectors/plugins/public/stripe/client/payments.go
Switched pagination HasMore reads from List().HasMore instead of ListMeta.HasMore in multiple Get* methods (empty-result early returns and final returns).
CurrencyCloud pagination loop
internal/connectors/plugins/public/currencycloud/balances.go
Refactored loop termination: changed for { ... if page < 0 { break } } to for page >= 0 { ... } moving the end condition to the loop header.
Duration / Timestamp serialization
internal/storage/migrations/utils.go, internal/storage/migrations/1-connector-configs.go, internal/storage/connectors.go
Changed duration JSON serialization to use d.String() and updated migration transform to use v2.PollingPeriod.String(); adjusted outbox idempotency key construction to use now.Format(...) instead of now.Time.Format(...).
Tests: model field access
internal/models/payment_adjustment_id_test.go, internal/models/payment_id_test.go
Updated assertions to access PaymentID top-level fields (Reference, Type) instead of nested PaymentReference fields to match decoding/struct changes.
Tests: control flow readability
internal/storage/outbox_test.go
Replaced if / else if connector ID checks with a switch over event.ConnectorID cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through flakes and bumped Go to one-two-six,
Looped less, read HasMore straight, and trimmed some nested tricks.
Durations now speak plainly, timestamps neatly lined,
Tests adjusted, devbox tuned — a tidy rabbit mind. 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title directly summarizes the main objective: upgrading Go from version 1.24 to 1.26, which is reflected across multiple files (flake.nix, dev.Dockerfile).
Description check ✅ Passed The description is related to the changeset, covering the main upgrade objectives and secondary improvements like the stable/unstable nixpkgs split and allowUnfreePredicate changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/upgrade-go
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sylr sylr force-pushed the chore/upgrade-go branch from 9f0b45d to ccf4b44 Compare March 20, 2026 10:07
- Run go mod tidy with Go 1.26 to resolve dependency versions correctly
- Fix genericclient module require path (v3 -> unversioned, matching replace)
- Add replace directive for gopkg.in/go-jose/go-jose.v4 (Go 1.26 enforces
  module path consistency)
- Add .golangci.yml for golangci-lint v2 compatibility (suppress ST* style
  checks and standard error handling exclusions to match v1 behavior)
- Apply Go 1.26 vet auto-fixes (redundant method receivers, loop refactoring)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 20, 2026

Codecov Report

❌ Patch coverage is 70.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.08%. Comparing base (0ac04d0) to head (ff23442).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...onnectors/plugins/public/stripe/client/accounts.go 0.00% 2 Missing ⚠️
.../plugins/public/stripe/client/external_accounts.go 0.00% 2 Missing ⚠️
...onnectors/plugins/public/stripe/client/payments.go 50.00% 1 Missing ⚠️
internal/storage/migrations/utils.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #676   +/-   ##
=======================================
  Coverage   63.08%   63.08%           
=======================================
  Files         832      832           
  Lines       37635    37633    -2     
=======================================
  Hits        23742    23742           
+ Misses      12317    12316    -1     
+ Partials     1576     1575    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sylr sylr marked this pull request as ready for review March 20, 2026 10:39
@sylr sylr requested a review from a team as a code owner March 20, 2026 10:39
@@ -1,19 +1,18 @@
{
description = "A Nix-flake-based Go 1.24 development environment";
description = "A Nix-flake-based Go 1.26 development environment";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd also need to change the various docker files

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before I forget -- I'm OK with the rest of the PR, but we should also change the dockerfiles :)

The original form !(a && b && c) is clearer than !a || !b || !c.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sylr sylr force-pushed the chore/upgrade-go branch from 0591604 to ff23442 Compare March 20, 2026 13:03
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
flake.nix (1)

26-35: Centralize the nixpkgs import setup.

Lines 26-35 now define two inline import shapes. That is fine today, but future overlay or config changes are easy to apply to one set and miss on the other. A small helper would keep the shared contract in one place.

♻️ Example refactor
-            pkgs = import nixpkgs {
-              inherit system;
-              overlays = [ nur.overlays.default ];
-              config.allowUnfreePredicate = pkg: builtins.elem (nixpkgs.lib.getName pkg) [
-                "goreleaser-pro"
-              ];
-            };
-            pkgs-unstable = import nixpkgs-unstable {
-              inherit system;
-            };
+            mkPkgs = source: extra: import source ({
+              inherit system;
+            } // extra);
+
+            pkgs = mkPkgs nixpkgs {
+              overlays = [ nur.overlays.default ];
+              config.allowUnfreePredicate = pkg: builtins.elem (nixpkgs.lib.getName pkg) [
+                "goreleaser-pro"
+              ];
+            };
+            pkgs-unstable = mkPkgs nixpkgs-unstable {};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@flake.nix` around lines 26 - 35, Create a single helper that builds the
nixpkgs import configuration and use it for both imports instead of duplicating
inline calls: extract the shared arguments (system, overlays = [
nur.overlays.default ], and config.allowUnfreePredicate with the goreleaser-pro
check) into a function or attribute (e.g., mkNixpkgs or nixpkgsArgs) and then
replace the two occurrences of "import nixpkgs { inherit system; overlays = [
nur.overlays.default ]; config.allowUnfreePredicate = pkg: builtins.elem
(nixpkgs.lib.getName pkg) [ \"goreleaser-pro\" ]; }" and "import
nixpkgs-unstable { inherit system; }" to call that helper (pass a variant flag
or override for unstable) so both pkgs and pkgs-unstable are created from the
same shared configuration (referencing pkgs, pkgs-unstable, import nixpkgs,
import nixpkgs-unstable, overlays, config.allowUnfreePredicate, system).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@flake.nix`:
- Around line 26-35: Create a single helper that builds the nixpkgs import
configuration and use it for both imports instead of duplicating inline calls:
extract the shared arguments (system, overlays = [ nur.overlays.default ], and
config.allowUnfreePredicate with the goreleaser-pro check) into a function or
attribute (e.g., mkNixpkgs or nixpkgsArgs) and then replace the two occurrences
of "import nixpkgs { inherit system; overlays = [ nur.overlays.default ];
config.allowUnfreePredicate = pkg: builtins.elem (nixpkgs.lib.getName pkg) [
\"goreleaser-pro\" ]; }" and "import nixpkgs-unstable { inherit system; }" to
call that helper (pass a variant flag or override for unstable) so both pkgs and
pkgs-unstable are created from the same shared configuration (referencing pkgs,
pkgs-unstable, import nixpkgs, import nixpkgs-unstable, overlays,
config.allowUnfreePredicate, system).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2bb4b32c-942b-450c-9ac7-7ec9d159a669

📥 Commits

Reviewing files that changed from the base of the PR and between c9401e8 and 0591604.

⛔ Files ignored due to path filters (4)
  • .golangci.yml is excluded by !**/*.yml
  • flake.lock is excluded by !**/*.lock, !**/*.lock
  • go.mod is excluded by !**/*.mod
  • go.sum is excluded by !**/*.sum, !**/*.sum
📒 Files selected for processing (11)
  • flake.nix
  • internal/connectors/plugins/public/currencycloud/balances.go
  • internal/connectors/plugins/public/stripe/client/accounts.go
  • internal/connectors/plugins/public/stripe/client/external_accounts.go
  • internal/connectors/plugins/public/stripe/client/payments.go
  • internal/models/payment_adjustment_id_test.go
  • internal/models/payment_id_test.go
  • internal/storage/connectors.go
  • internal/storage/migrations/1-connector-configs.go
  • internal/storage/migrations/utils.go
  • internal/storage/outbox_test.go

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
@flemzord flemzord enabled auto-merge March 25, 2026 09:59
Copy link
Copy Markdown
Contributor

@laouji laouji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dirty check is failing. let's not merge because there is a diff in unexpected places

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants