substrate
substrate copied to clipboard
Move Throughput into `sc-sysinfo`
Prepares for #12017
I think it is also fine to split the issue into two MRs, depending on your preferences.
The changes here already make sense to have on their own.
I think it is also fine to split the issue into two MRs, depending on your preferences. The changes here already make sense to have on their own.
Sure, that makes sense.
I can review earliest on Friday. But maybe @koute wants to, it touches your code :)
You are only reafactoring code as preparation for #12017, right?
Then please update the issue description to not Close the issue.
We also need to ensure that this does not break telemetry.
You are only reafactoring code as preparation for #12017, right? Then please update the issue description to not Close the issue. We also need to ensure that this does not break telemetry.
Yes, this is only preparation for #12017, I changed the description now. The CI doesn't seem happy, I will do some minor fixes on this.
I also don't really get why we need Throughput there. @ggwpez can you give more context?
I also don't really get why we need
Throughputthere. @ggwpez can you give more context?
The measurement of the performance at startup runs in sc-sysinfo. So if we now want to compare the results of that to anything, we need the Throughput types to have nicer compare logic and formatting in the final output. The type only exists to make it possible to deserialize from JSON with a unit suffix.
I just originally did not put it in sy-sysinfo to keep the sc- crate small, since it was not needed.
Should I move Metric also? Or should that be done in a separate PR?
Should I move
Metricalso? Or should that be done in a separate PR?
Let's do that in a separate PR.
@ggwpez So what is the final decision then?
@ggwpez So what is the final decision then?
Okay then lets remove the unit from the serialization. Less code - less bugs.
Okay then lets remove the unit from the serialization. Less code - less bugs.
@ggwpez What should we do with reference_hardware.json then?
Edit: wouldn't auto-generating it result in even more code?
@ggwpez What should we do with
reference_hardware.jsonthen?
Change it so that it works with the new format.
Edit: wouldn't auto-generating it result in even more code?
We need auto-generation at one point anyway, as manual editing is error-prone and not straight forward.
@ggwpez So I assume we should use MiBs in the reference_hardware.json then?
@ggwpez can we merge this?
bot merge
Waiting for commit status.