Closed schnerring closed 4 years ago
Very nice! Thank you very much for your hard work. I especially appreciate that you tested your code with the shellcheck tool.
Some background: You are quite skilled at shell programming. I'm just an old C/C++ programmer and shell scripts are not my forte. The only reason I wrote this one (and the others here on GitHub) was to contribute, in a small way, to the FreeNAS project, and to make my own life easier.
I see you added a copyright notice. Your contributions could be construed as a 'derivative work' and therefore ineligible for copyright protection. But I'm not construing it that way and I don't mind -- as long as you don't intend to sell this tool. My intent is for it to be freely available to everyone. Do we agree on that issue?
I will try to answer your questions:
What's the point being able to specify two separate log directories?
I honestly don't remember why I did that! I'm sure it was a valid reason at the time. I like your simpler approach.
What's the point of using 'sda' as a script option instead of the whole path '/dev/sda'? This seems kind of atypical compared to other shell commands.
I've tested over 200 drives with this script... and constantly having to enter the '/dev/' path seemed redundant after awhile. The gpart program, for one example, doesn't require the '/dev/' prefix.
What's the point of "cleaning up" the logs at the very end? What struck me as odd was removing the copyright notice of smartctl. Does that info really hurt?
I was trying to eliminate superfluous output from the script to make it a little easier to read.
What was the reason to change the script to run in non-dry mode by default? I feel like that's pretty dangerous.
The most common complaint I've heard from users is "Why does the script default to dry-run mode? I don't want to have to edit the script!". And then, when they do edit it, they use a Windows editor that introduces CR/LF line endings. Argh!
Renaming all read-only constants to uppercase (5d702df) is opinionated but I find Google's styleguide a very good approach to streamline shell script code. What do you think?
I don't have a strong opinion about it, either way, except for a residual dislike for uppercase from my old FORTRAN/BASIC days. And it can seem like the code is SHOUTING at me!
I do like consistency and a uniform style when it comes to variable naming, indentation, etc., so I am happy to follow this convention.
Thanks for the quick response :)
I especially appreciate that you tested your code with the shellcheck tool.
I first heard about shellcheck
in when reading the comments in PR#6, so thanks for introducing me. I use a VS Code extension that comes bundled with the binary and makes life much, much easier.
I see you added a copyright notice. Your contributions could be construed as a 'derivative work' and therefore ineligible for copyright protection. But I'm not construing it that way and I don't mind -- as long as you don't intend to sell this tool. My intent is for it to be freely available to everyone. Do we agree on that issue?
IANAL: I agree with you. Licensing is just one of those things that I can't wrap my head around. I try to learn but most of the time I don't know what I'm doing, honestly. I pretty much blindly followed the recommendations in this post: How to manage a copyright notice in an open source project?
I'm just trying to "do it right", whatever that is. I intend to publish my "stress test USB stick creator tool" on GitHub under MIT but since it downloads various tools during creation licensing keeps me from actually publishing it, yet, because I'm technically distributing that software, I think.
I've tested over 200 drives with this script... and constantly having to enter the '/dev/' path seemed redundant after awhile. The gpart program, for one example, doesn't require the '/dev/' prefix.
My guess would be that it allows both variants, something to add on my TODO list.
The most common complaint I've heard from users is "Why does the script default to dry-run mode? I don't want to have to edit the script!". And then, when they do edit it, they use a Windows editor that introduces CR/LF line endings. Argh!
Something .editorconfig improves :) Note, I have changed the behavior back to running in dry-run mode by default. -f
has to be provided. I don't want your users complaining again, so shall I change that back?
I'm gonna open a new PR for the items still TODO.
First of all, awesome script, thanks so much for putting it together!
I found this on the iXsystems Community forum and use it on my custom Tiny Core Linux stress test thumb drive to burn-in my FreeNAS system. During testing I've come to understand the bits and pieces of this script more and more but I found some parts pretty hard to understand as a shell scripting newbie.
Some of my changes are inspired by PR#6 but POSIX compliant. I used this opportunity to further my shell scripting abilities.
I'm running this version of the script on four Seagate Iron Wolf 12 TB (
ST12000VN0008
) disks on Tiny Core Linux.Changes
Looking at all changes at once is too much but I aimed to make the changes in each commit self-explanatory. I used multiline commit messages when changing multiple, related things at once.
New Features
-h
help text; only run in non-dry mode when-f
is provided for safety reasons.editorconfig
to streamline editor behavior for developmentRefactoring
tr
andpcregrep
by just usingawk
printf
instead ofecho
to improve portabilityQuestions
-o <dir>
optionsmartctl
. Does that info really hurt?TODO
# ACTION BEGINS HERE
)badblocks
README.md
and script header to reflect changessda
and/dev/sda
for<disk>