Add a multi-file based target description mechanism #2226
Conversation
Yatekii
force-pushed
the
task/target-yaml-v2
branch
3 times, most recently
from
35c1c71
to
41533c3
Compare
Yatekii
force-pushed
the
task/target-yaml-v2
branch
from
41533c3
to
43849db
Compare
|
I am not sure what exactly we want to do with the new format. I would like it actually if maybe we could have the extracted flash algo binary in a separate file. |
Yatekii
force-pushed
the
task/target-yaml-v2
branch
2 times, most recently
from
521f92f
to
5637db8
Compare
There was a problem hiding this comment.
This feels like a solution looking for a problem. So far the only case where I personally could have used something like this was the ESP32 support, where two devices are extremely close, except for suble differences. But in the end we ended up not adding two different variants there.
I'm not sure how much value there is in just splitting existing descriptions up into pieces. If a family grows too big, it can already be broken up into separate descriptions. Is there enough reuse potential to justify complicating things?
| ]; | ||
|
|
||
| let merged = ChipFamily::merge(values).unwrap(); | ||
| insta::assert_yaml_snapshot!(merged); |
There was a problem hiding this comment.
Does this have to be piped through insta? I wouldn't think comparing two 4-line yaml blobs really wants a solution where the expectation is in a different folder.
There was a problem hiding this comment.
I like it because it makes spotting differences really nice and manageable. It is a dev dependency, so it should not matter much.
|
We have many target files that we have patched manually. And it happened many times that someone regenerated from a pack and we had a regression due to that. |
Yatekii
force-pushed
the
task/target-yaml-v2
branch
5 times, most recently
from
22e4184
to
5656507
Compare
Yatekii
force-pushed
the
task/target-yaml-v2
branch
from
5656507
to
76aac45
Compare
| @@ -253,10 +255,13 @@ async fn cmd_arm(out_dir: Option<PathBuf>, chip_family: Option<String>, list: bo | |||
| let mut generated_files = Vec::with_capacity(families.len()); | |||
|
|
|||
| for family in &families { | |||
| let path = out_dir.join(family.name.clone().replace(' ', "_") + ".yaml"); | |||
| let file = std::fs::File::create(&path) | |||
| let path = out_dir.join(family.name.clone().replace(' ', "_")); | |||
There was a problem hiding this comment.
Nitpick, do we need to clone here?
No description provided.