cki-project / sktm

"Sonic Kernel Testing Manager" - a job scheduler for skt
GNU General Public License v2.0
0 stars 6 forks source link

Improve argparse formatting #120

Closed danghai closed 6 years ago

danghai commented 6 years ago

I make PR to improve argparse formatting.

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 416


Changes Missing Coverage Covered Lines Changed/Added Lines %
sktm/executable.py 0 21 0.0%
<!-- Total: 0 21 0.0% -->
Files with Coverage Reduction New Missed Lines %
sktm/executable.py 3 16.13%
<!-- Total: 3 -->
Totals Coverage Status
Change from base Build 407: 0.04%
Covered Lines: 465
Relevant Lines: 1122

💛 - Coveralls
veruu commented 6 years ago

Thanks Hai. Can you explain (also in the commit message) how stretching a mostly single line of code into 4-6 improves formatting? I'd understand if you changed only ones that don't fit, to see the parameters clearly, but I don't get the reason behind changing the single lines.

danghai commented 6 years ago

@veruu I think making them the same format is good. It is clear, and more readabe. It looks the same as skt. I do the same this pull: https://github.com/RH-FMK/skt/pull/139/files

veruu commented 6 years ago

The point of the skt pull you linked was to unite the subparsers (which is not needed in sktm), and the format change was a side effect that was questionable. The consistency argument made sense in skt because most of the parameters didn't fit the single line but that's not applicable here. Don't fix what's not broken ;)

danghai commented 6 years ago

@veruu ah okay. So I close this PR now