[Merged by Bors] - add get_history function to Diagnostic #2772
Conversation
| @@ -119,6 +119,10 @@ impl Diagnostic { | |||
| pub fn get_max_history_length(&self) -> usize { | |||
| self.max_history_length | |||
| } | |||
|
|
|||
| pub fn get_history(&self) -> Vec<f64> { | |||
There was a problem hiding this comment.
| pub fn get_history(&self) -> Vec<f64> { | |
| pub fn history(&self) -> Vec<f64> { |
Elsewhere in the code base we tend to prefer this naming convention.
|
Two quick questions about the return type:
I think that two separate methods make sense here; one for just the values, called |
alice-i-cecile
added
C-Usability
|
Although i haven't had a use for it yet, I think your completely right. Just to be clear you are talking about two functions signatures like this right? pub fn values(&self) -> impl Iterator<Item=&f64>;
pub fn history(&self) -> impl Iterator<Item=&DiagnosticMeasurement> |
|
Yup I agree that we should be using iterators here instead of allocating. pub fn values(&self) -> impl Iterator<Item=&f64>;
pub fn measurements(&self) -> impl Iterator<Item=&DiagnosticMeasurement>Using "measurements" also has the benefit of having parity with |
Correct :) I also prefer Cart's |
alice-i-cecile
added
the
A-Diagnostics
|
Sorry for taking a while to respond, I agree with all the points made and have made the changes. I believe this should be ready for a final look then |
|
bors r+ |
# Objective - Allow access to diagnostic history value. - Fixes #2771. ## Solution - Add Diagnostic::get_history function.
|
Haha just realized this will fail. @ByteBuddha can you run |
|
Build failed: |
|
bors r+ |
# Objective - Allow access to diagnostic history value. - Fixes #2771. ## Solution - Add Diagnostic::get_history function.
bors
bot
changed the title
Objective
Solution