Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: change default yaml flow style to False #11

Merged
merged 3 commits into from
Apr 3, 2024
Merged

feat: change default yaml flow style to False #11

merged 3 commits into from
Apr 3, 2024

Conversation

RadxaYuntian
Copy link
Contributor

The default flow style is hard to read for non-trivial system configurations.

Here is how different flow style looks like.

This is a real output we are currently generating with bdebstrap, which is hard to read: config.yaml.txt

Readability is important because we have customers who want to customize our systems. In the past we have to teach them how to use our build system, but we plan to just let them make changes to the generated config.yaml and build it.

Extension name changed to pass GitHub upload rule.

@RadxaYuntian RadxaYuntian marked this pull request as draft February 6, 2024 03:06
Copy link

codecov bot commented Feb 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.16%. Comparing base (6e323c4) to head (01d6696).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #11      +/-   ##
==========================================
+ Coverage   97.13%   97.16%   +0.02%     
==========================================
  Files          12       12              
  Lines         873      882       +9     
  Branches        8        8              
==========================================
+ Hits          848      857       +9     
  Misses         23       23              
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RadxaYuntian RadxaYuntian marked this pull request as ready for review February 6, 2024 03:11
@bdrung
Copy link
Owner

bdrung commented Feb 6, 2024

The change looks good. Can you add a test case for this change?

@RadxaYuntian
Copy link
Contributor Author

I'm currently in vacation. I'll revisit this PR after 18th.

The default flow style is hard to read for non-trivial system configurations.

Signed-off-by: ZHANG Yuntian <[email protected]>
@RadxaYuntian
Copy link
Contributor Author

Finally got time around for this one again. @bdrung please take a look.

.gitignore Show resolved Hide resolved
@RadxaYuntian RadxaYuntian requested a review from bdrung April 3, 2024 03:10
@bdrung
Copy link
Owner

bdrung commented Apr 3, 2024

Thanks.

@bdrung bdrung merged commit 0748a91 into bdrung:main Apr 3, 2024
16 checks passed
@RadxaYuntian RadxaYuntian deleted the patch-1 branch April 3, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants