[tech] move the enhance co2 function out of the model #738
Conversation
This is the first step to split `model.rs` into several files to improve maintanability.
antoine-de
changed the title
src/enhancers/enhance_co2.rs
Outdated
|
|
||
| /// Physical mode should contains CO2 emissions. If the values are not present | ||
| /// in the NTFS, some default values will be used. | ||
| pub fn enhance_with_co2(collections: &mut Collections) { |
There was a problem hiding this comment.
I'm never sure which is better between
pub fn enhance_with_co2(collections: &mut Collections) { ... }and
pub fn enhance_with_co2(collections: Collections) -> Collections { ... }The benefit of the second version is that the function is pure.
There was a problem hiding this comment.
sure but I wanted to have minimal changes and I'm not really convinced one is really better than the other 🤔
There was a problem hiding this comment.
And you were probably right to do minimal changes at this point. I guess I took the opportunity to review it since the code was changed. Anyone else has an opinion on the topic?
There was a problem hiding this comment.
It looks OK, although I think @woshilapin and @datanel should have a look at the new storage.
Detail (if I'm not mistaken): The order in which enhancements/cleanups are applied matters for performance and functionnality, so we have to pay attention to have this readable and accessible I guess. Maybe just pay attention to enhancers' rust-comments is doing most of the job.
antoine-de
force-pushed
the
model_split
branch
from
d5709ad to
3d32142
Compare
antoine-de
dismissed stale reviews from ArnaudOggy and woshilapin
via
bc83e03
antoine-de
force-pushed
the
model_split
branch
from
3d32142 to
bc83e03
Compare
|
You need to bump the |
antoine-de
dismissed stale reviews from pbougue, ArnaudOggy, and woshilapin
via
e3e696f
This is the first step to split
model.rsinto several files to improve maintanability.To ease review I think you can review this and if it's ok we can merge and do separate PR