-
Notifications
You must be signed in to change notification settings - Fork 134
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
Neosync Interval and Interval Array type #2993
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2993 +/- ##
==========================================
+ Coverage 35.57% 35.77% +0.20%
==========================================
Files 337 342 +5
Lines 39120 39529 +409
==========================================
+ Hits 13915 14141 +226
- Misses 23574 23729 +155
- Partials 1631 1659 +28 ☔ View full report in Codecov by Sentry. |
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.
Overall this looks great.
Left a few small comments.
Beginning of a new system!
return v | ||
default: | ||
// Check if the type implements Value() method | ||
if valuer, ok := v.(neosynctypes.NeosyncAdapter); ok { |
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.
Could further slim this down by having a NeosyncPgxValuer()
interface that only calls out the ValuePgx
function. I think that's all this thing needs anyways.
@@ -189,12 +201,18 @@ func (g *neosyncInput) Read(ctx context.Context) (*service.Message, service.AckF | |||
}, nil | |||
} | |||
} | |||
|
|||
registry := neosynctypes.NewTypeRegistry(g.logger) |
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.
Should this be registered every time or just during init?
valuesMap := map[string]any{} | ||
for col, val := range row { | ||
if len(val) == 0 { | ||
valuesMap[col] = nil | ||
} else { | ||
valuesMap[col] = val | ||
newVal, err := registry.Unmarshal(val) |
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.
so clean
This changes interval type in javascript transformer to an object instead of a string