salt icon indicating copy to clipboard operation
salt copied to clipboard

grains: Split os_data into smaller functions

Open bdrung opened this issue 3 years ago • 2 comments

The os_data function is huge (531 lines for one function is too long) and also deeply nested (8 levels deep). That make reading, modifying, and testing this function harder.

Split os_data into smaller functions to a readable 167 lines with only a nesting level of three.

os_data has a case-like statement to differentiate the different operating systems, but this is split into two parts.

To make the code more readable, combine all case-like statement in os_data to use one long if-elif chain.

bdrung avatar Feb 07 '22 22:02 bdrung

Rebased and added two more commits. os_data is now much cleaner and readable.

bdrung avatar Feb 09 '22 17:02 bdrung

Rebased and added a test case for _linux_lsb_distrib_data().

bdrung avatar Feb 11 '22 11:02 bdrung

@dmurphy18 https://github.com/saltstack/salt/pull/61626 is the final merge request containing all changes. Since that merge request is big, I split it into multiple smaller chunks (that stack on each other). This merge request is the first one. I rebased it on master and resolved the conflict.

bdrung avatar Sep 22 '22 20:09 bdrung

Why does this pull request need a changelog entry? It does not change anything for the user. Only https://github.com/saltstack/salt/pull/61626 has user facing changes and changelog entries.

bdrung avatar Sep 23 '22 23:09 bdrung

@bdrung Sorry, so used to everything needing a changelog. You are correct, if it did not impact user and was only internal things it does not add a changelog, approving this

dmurphy18 avatar Sep 26 '22 16:09 dmurphy18

Thanks. The next pull request in this series is https://github.com/saltstack/salt/pull/61589

bdrung avatar Sep 26 '22 22:09 bdrung