refactor: initialize config structs with default values - #1090
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1090 +/- ##
==========================================
+ Coverage 41.27% 41.58% +0.32%
==========================================
Files 21 21
Lines 1820 1816 -4
==========================================
+ Hits 751 755 +4
+ Misses 1069 1061 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2181485 to
27896eb
Compare
orhun
left a comment
There was a problem hiding this comment.
Nice! The config loading is pretty convenient, I added a comment about that.
|
Uh-oh, fixtures seems to be broken 😵 |
5bde6fb to
97020d8
Compare
97020d8 to
dfe9712
Compare
8cc2acd to
a29045d
Compare
orhun
left a comment
There was a problem hiding this comment.
Just to double check: the fixture configs has been trimmed because we now have those values in the default config, right?
Yes exactly. Imho duplicating default values over and over again is bad practice because it makes it difficult to find differences. |
a29045d to
e61a31f
Compare
Description
This change uses the embedded configuration to initialize configuration options with default values whenever possible. This moves the concern of defining default values into the embedded config and reduces complexity when using the configuration.
Motivation and Context
Closes #1087.
Using the embedded config.toml to initialize the config structs allows us to use well defined defaults in the code where previously we had to repeatedly unwrap
Option<>in lots of places. The embedded config.toml was chosen because it is the config used by git-cliff itself and should follow all of our best practices.This change also allowed for minimizing the fixtures' config files. Only values that are different to the defaults remain. Thus fixtures are less cluttered and easier to maintain.
How Has This Been Tested?
Regression testing was done using the existing fixtures and integration tests.
Screenshots / Logs (if applicable)
Types of Changes
Checklist: