clearlinux / clr-installer

Clear Linux* OS Installer
GNU General Public License v3.0
92 stars 42 forks source link

Cleanup of storage code #686

Closed karthikprabhuvinod closed 4 years ago

karthikprabhuvinod commented 4 years ago

1.) Move some code to test so that they are not part of executable 2.) Remove unused code

Signed-off-by: Karthik Prabhu Vinod karthik.prabhu.vinod@intel.com

Please ensure the above guidelines for contributing are met.

530

Changes proposed in this pull request: 1.) Move some code to test so that they are not part of executable 2.) Remove unused code

karthikprabhuvinod commented 4 years ago

@mdhorn Okay. ya you are correct, The code is run from test runner and not reachable through executable's main. But still, the code should be in Test files correct? Only the code which is required for executable should be in storage.go and ops.go.

https://github.com/clearlinux/clr-installer/pull/686#pullrequestreview-380470152

I think its okay to include test helper functions inside test files and they dont have to be _test functions.

mdhorn commented 4 years ago

@karthikprabhuvinod Okay, here is the history; these functions were not created for driving test cases but were written for use by the TUI when the TUI had a built-in interface for disk partitioning. For reference, you can use git blame to see where/why the code was added

git blame storage/storage.go
a58eb66e28d12 (Mark D Horn     2018-11-18 12:48:40 -0800  971) func IsValidLabel(label string, fstype string) string {
git show a58eb66e28d12

So, if these functions are truly not being used by anything in the main code, and are only exercised by test code, then we should drop the functions and the associated tests.