oicr-gsi / robust-paper

Ten Simple Rules for Making Research Software More Robust. Manuscript is published at PLoS Computational Biology. Feedback is welcome!
https://doi.org/10.1371/journal.pcbi.1005412
MIT License
12 stars 0 forks source link

Itemizing feedback #21

Closed gvwilson closed 7 years ago

gvwilson commented 7 years ago

I've taken @morgantaschuk's summary and my own notes and itemized the fixes I think we need to make (at the top of the .tex file), as well as fixing a handful of minor items in the main text.

morgantaschuk commented 7 years ago

One point about the reviews that struck me is that we're somewhat divided between "good for production/other people/contributions" and "minimal effort in an otherwise finished script". The two aren't necessarily compatible, and often the kind of changes that people suggested are very application-specific or would require major re-writes of their code (like 'micro-services' style applications and minimizing code complexity, use 'stable' versions of languages, environment variables vs. config files). I lean more towards the former definition than the latter, but if that's the case, I'm less happy calling them "rules". More like "guidelines", though that's not as catchy.

gvwilson commented 7 years ago

I agree we should lean toward "good for production"; I also agree that "rules" may not be the best term, but it's what the series we're hoping to submit to uses as a tag...

morgantaschuk commented 7 years ago

``Write documentation'' is not one of our ten rules. Explain in the conclusion that we think this is really important, but it's not the software per se.

We already suggest that people write a README, so adding documentation isn't that much of a stretch. Since we're combining versioning with past versions, it makes sense to add a short documentation section.

Find somewhere to say, ``Check that all input values are in a reasonable range, choose reasonable defaults where possible, and no defaults at all otherwise.''

Maybe either in 8. Parameters, or expand 7. No hard-coding to include parameters as well as paths.

Explain somewhere that there's an 11th rule for compiled languages: use standard build tools like Make rather than home-brewed scripts.

A slight re-work of 6. No root perhaps. "Minimize the time and permissions needed to compile and use the software". I kind of forgot about compilation because I've been using scripts or polite build tools like Maven for so long. I had to compile a C++ program just last week and tracking down all of the dependencies, running make multiple times, fixing missing system dependencies, working around dynamically linked libraries, and handling the slight differences between operating systems took half of my day just to be able to launch the program for the first time. And that's a relatively quick turn-around.

Should we tell people to create several single-purpose tools rather than One Script to Rule Them All?

In the testing section, we can mention that smaller, self-contained Hobbits tools are easier to test than Sauron-sized scripts, and if you're having problems writing tests, splitting them into functions might help (and make the function clearer too).

Should we tone down the ``tremor of fear'' remark?

Probably, although it is a hook.

What's the relationship here between parameter and feature?

Parameters are a method to access software features. An example is setting a proxy on a browser. It's not necessarily required for it to function, but some locations need it, and so it has to be able to be turned on and turned off at will. The proxy is the feature, and the parameter is however you set it, whether through command-line or Settings menu.

Honor local conventions, e.g., use \texttt{TMPDIR} for temporary files. (The problem is, where are all of these written down?)

Some HPCs won't let you use /tmp either. If by 'local' convention you mean "unique to my organization", then code review is the only real way to determine them (unless your lab has extensive coding guidelines and code review checklists like C Titus Brown's, but in that case you're probably way ahead of this particular set of rules).

Combine versioning and making previous versions available into a single point in order to free up space for something else to be rule 10?

Yes.

morgantaschuk commented 7 years ago

Shoot, wrong button.

gvwilson commented 7 years ago

@morgantaschuk I didn't know what to do about three of the feedback items, but the rest have been incorporated - please merge if you like the changes, and if you can figure out what to do about incorporating the references and merging the two version control points, please let me know.