chore: simplify, modernize (Go 1.26), update deps#148
Conversation
- metrics: *uint64 fields → atomic.Uint64 value fields; remove new(uint64(0)) init - pipeline: extract toInt64 helper to deduplicate Int/Priority type-switch; remove dead ParseInt bounds guard - job: add nil-Options guards in Delay/AutoAck - plugin: fix Declare to return error when constructor is missing; drop redundant len>0+index-range; use var for jst slice - worker_pool: rename context field → ctx (shadowed import); clear+reslice errs instead of reallocating - rpc: remove redundant per-call mutex (delegates to Plugin methods that own their lock) - commands: use pipe.Has(createdWithConfig) instead of String(...) != ""
|
Warning Review limit reached
More reviews will be available in 24 minutes and 1 second. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR modernizes the jobs plugin for Go 1.26, simplifies several internal helpers, updates dependencies, and fixes a few small correctness issues around nil job options and dynamic pipeline declaration errors.
Changes:
- Modernizes metrics counters to use
atomic.Uint64and updates Go/dependency versions. - Simplifies pipeline/job processing code and command handling.
- Adds safer behavior for nil
Job.Optionsand missing driver constructors duringDeclare.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
worker_pool.go |
Renames context field and reuses the processor error slice when clearing errors. |
rpc.go |
Removes RPC-level locking around delegated plugin operations. |
plugin.go |
Simplifies consume initialization and makes missing dynamic driver constructors return errors. |
pipeline.go |
Deduplicates numeric conversion logic for Int and Priority. |
metrics.go |
Replaces pointer-based atomic counters with atomic.Uint64 fields. |
job.go |
Adds nil Options guards for Delay and AutoAck. |
commands.go |
Uses Pipeline.Has for config-created pipeline detection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
On 32-bit platforms int is 32-bit, so casting an unchecked int64 from toInt64 directly to int could silently truncate values that fit in int64 but not int. Add math.MinInt/math.MaxInt bounds check before the cast so out-of-range values fall through to the default instead of wrapping. Resolves CodeQL "Incorrect conversion between integer types" finding.
Applied fixes
Simplify (S) — 5 applied
commands.go:pipe.String(createdWithConfig, "") != ""→pipe.Has(createdWithConfig)pipeline.go: deduplicateInt()/Priority()numeric type-switch into sharedtoInt64helperworker_pool.go:errors()clears slice withclear+ reslice instead of reallocatingplugin.go: drop redundantlen(Consume)>0guard; usevar jstinstead ofmake([]*State,0,2)rpc.go: removerpc.mu— the RPC methods delegate straight toPluginwhich manages its own lockModernize (M) — 3 applied
metrics.go:*uint64fields →atomic.Uint64value fields; dropnew(uint64(0))init; use.Add(1)/.Load()worker_pool.go: renamecontext context.Contextfield →ctx(shadowed thecontextimport)plugin.go: index-range overcfg.Consume→ value rangeReview / bugs (R) — 3 applied
pipeline.go: remove dead post-ParseInt(...,32)bounds guard (ParseIntalready enforces int32 range)job.go: add nil-Optionsguards inDelay()andAutoAck()(would panic on a job without options)plugin.go:Declarenow returns an error when no constructor is registered for the requested driver (previously returned nil silently)