YAML consolidation prelude: new YAML dump APIs (FR-702) #251
Conversation
|
Draft as it depends on #250 |
schopin-pro
changed the title
schopin-pro
force-pushed
the
yaml-consolidation-api-prelude
branch
from
c68271a
to
b605144
Compare
schopin-pro
force-pushed
the
yaml-consolidation-api-prelude
branch
from
b605144
to
6920acc
Compare
Codecov Report
@@ Coverage Diff @@
## main #251 +/- ##
==========================================
+ Coverage 99.12% 99.13% +0.01%
==========================================
Files 60 60
Lines 10229 10404 +175
==========================================
+ Hits 10139 10314 +175
Misses 90 90
Continue to review full report at Codecov.
|
schopin-pro
force-pushed
the
yaml-consolidation-api-prelude
branch
2 times, most recently
from
bb22c20
to
086c4c8
Compare
schopin-pro
force-pushed
the
yaml-consolidation-api-prelude
branch
2 times, most recently
from
4744eb7
to
2496e6f
Compare
There was a problem hiding this comment.
Thank you Simon! This is another great step in consolidating our YAML parsing story.
It is looking very good overall. I asked for some clarification and mentioned a few nitpicks in the inline comments. We should be good to merge this after those are resolved!
Also, I can confirm that the netplan_state_write_yaml() function isn't used anywhere and was never released. So we're good to "break" that API.
This allows for more flexibility than using filenames. The implementation is split in two functions to allow for future variants, i.e. updating an existing netplan configuration tree. This patch is technically an API break but the removed API was never released. V2: * use a helper function netplan_state_has_nondefault_globals() * fix the write_netplan_conf_full behavior on empty state to not create an empty file * DRASTICALLY simplify the implementation of netplan_state_dump_yaml(), fixing a leak by not using a new list in the first place
schopin-pro
force-pushed
the
yaml-consolidation-api-prelude
branch
from
2496e6f
to
2815606
Compare
This API is also using file descriptors for flexibility. The idea is to be able to copy only part of a YAML document based on a "prefix", that is subsequent keys into YAML maps, separated by '\t' characters. V2: add some comments and use better names for the static helpers
schopin-pro
force-pushed
the
yaml-consolidation-api-prelude
branch
from
2815606
to
b10ff76
Compare
Description
Introduce a couple of APIs that are needed for the various YAML consolidation efforts. Note that this is technically an API break, but the function that is removed has never been part of a release, so this should be OK.
Checklist
make checksuccessfully.make check-coverage).