-
Notifications
You must be signed in to change notification settings - Fork 13
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
Ebonilla/sy 749 opc ua read task cleanup and tests #640
Ebonilla/sy 749 opc ua read task cleanup and tests #640
Conversation
…sy-693-support-for-cesium-writer-err-on-unauthorized
…nto 585-sy-693-support-for-cesium-writer-err-on-unauthorized
…nto 585-sy-693-support-for-cesium-writer-err-on-unauthorized
…d' of https://github.com/synnaxlabs/synnax into ebonilla/sy-749-opc-ua-read-task-cleanup-and-tests
…nto ebonilla/sy-749-opc-ua-read-task-cleanup-and-tests
…nto ebonilla/sy-749-opc-ua-read-task-cleanup-and-tests
…la/sy-749-opc-ua-read-task-cleanup-and-tests
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## rc #640 +/- ##
=====================================
Coverage ? 51.64%
=====================================
Files ? 1040
Lines ? 78517
Branches ? 3249
=====================================
Hits ? 40554
Misses ? 36988
Partials ? 975
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…la/sy-749-opc-ua-read-task-cleanup-and-tests
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.
Good after some minor changes
func Open(configs ...Config) (db *DB, err error) { | ||
cfg, err := config.New(DefaultConfig, configs...) | ||
if err != nil { | ||
return nil, err |
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.
wrap
c, err := controller.New[*controlEntity](controller.Config{ | ||
Concurrency: control.Shared, | ||
Instrumentation: cfg.Instrumentation, | ||
}) | ||
if err != nil { | ||
return nil, err |
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.
wrap
@@ -20,6 +21,7 @@ var _ = Describe("Write", func() { | |||
DataType: telem.TimeStampT, | |||
Virtual: true, | |||
}, | |||
FS: xfs.NewMem(), |
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.
Why only mem FS?
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.
FS implementation is still checked for in the config but does not apply to any test cases. Feel free to adjust as you build out future test suites
client/cpp/framer/frame.cpp
Outdated
@@ -59,9 +59,9 @@ void Frame::toProto(api::v1::Frame *f) const { | |||
|
|||
// implement th ostream operator |
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.
typo: the
@@ -170,9 +170,9 @@ class Streamer { | |||
}; | |||
|
|||
enum WriterMode : uint8_t { |
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.
Do we still need WriteMode if we got rid of SetMode?
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.
Yes, you can set the mode when opening the writer
} | ||
|
||
|
||
std::pair<Task, freighter::Error> TaskClient::retrieveByType( |
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.
why are some functions snake case and some functions camel case?
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.
We need to standardize the convention we use. I'll leave this to @Lham42 as a chore
time_range=sy.TimeRange( | ||
start=above_threshold, end=value["daq_time"][-1] | ||
), | ||
color="#BADA55", |
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.
interesting hex
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.
Ha
…la/sy-749-opc-ua-read-task-cleanup-and-tests
No description provided.