-
Notifications
You must be signed in to change notification settings - Fork 106
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
feat: integrating numscript rewrite #503
Conversation
71f557e
to
51f4c7b
Compare
aeb468e
to
75d32fc
Compare
76ef6f9
to
7f97a02
Compare
19412d8
to
e2ed5b9
Compare
e2ed5b9
to
9d4e597
Compare
318f531
to
2725fb3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #503 +/- ##
==========================================
- Coverage 82.79% 82.75% -0.05%
==========================================
Files 117 117
Lines 6773 6860 +87
==========================================
+ Hits 5608 5677 +69
- Misses 881 898 +17
- Partials 284 285 +1 ☔ View full report in Codecov by Sentry. |
5133f60
to
594e139
Compare
a96a496
to
72e6de6
Compare
}) | ||
When("creating a bulk on a ledger", func() { | ||
var ( | ||
now = time.Now().Round(time.Microsecond).UTC() |
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.
Not a big deal but there is wrapper around time
package in go-libs, which already round dates at microseconds (postgres precision)
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.
and set UTC too
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.
Approved but i would be grateful if you can change the decorating stuff around the parser.
Keep the option simple, and make the plumber in the module.go file.
56191fd
to
b4fd1b9
Compare
wrapping all tests into a loop that runs once with the old impl, and once with the new one
b4fd1b9
to
056b89c
Compare
a148911
to
bc861fc
Compare
}{ | ||
{"default", false}, | ||
{"numscript rewrite", true}, | ||
} { |
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.
Here the only diff is that I wrapped everything in this for loop
Debug: debug, | ||
NatsURL: natsServer.GetValue().ClientURL(), | ||
BulkMaxSize: bulkMaxSize, | ||
ExperimentalNumscriptRewrite: data.numscriptRewrite, |
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.
Not that here it's using the feature flag value
}{ | ||
{"default", false}, | ||
{"numscript rewrite", true}, | ||
} { |
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.
Same as before, here the only diff is wrapping all in a loop
Changelog:
The history is a bit messed up after the last rebase I did but should mostly make sense so you can read commit by commit