SciKit-Surgery / scikit-surgery

SciKit-Surgery - Compact Libraries for Surgical Navigation
http://scikit-surgery.github.io/scikit-surgery/
Other
40 stars 11 forks source link

Question on code duplication in docs, unit tests, etc. #17

Closed thompson318 closed 4 years ago

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 6, 2018, 11:17

If we have requirements.rst for software requirements, then we document code with docstrings, then we have unit tests, which also define/validate the spec, then do we have too much duplication? What's a clean simple set of advice here?

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 23, 2018, 23:25

assigned to @StephenThompson

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 26, 2018, 09:56

I tried editing requirements.rst. I was trying to make it so I could refer to pages in the API docuemntation such as within comments of the docstring of either the functions implementing the functionality, or the functions that were the unit tests. I couldn't do it.

So, if a developer writes excellent API documentation, and also, the unit test has a self explanatory name, and very succinctly summarises what the functionality is, is that enough?

The requirements.rst could be a document that is good practice to start at the beginning of development. Then as you implement things, your API code and unit tests should be sufficient, at least to a developer.

And/Or, given that we are doing medical device development, is it envisaged that requirements.rst is updated in addition to the above levels of documentation. I presume so, and I agree that it should always be in sync, and that validating that your unit tests do actually test your requirements. But in practice, this might be hard if we have multiple developers, creating multiple modules, each with independently numbered unit tests, and the core SNAPPY team do not want to be encumbered with maintaining these things.

So, should the aim be:

a) We publicise requirements.rst and the best practice, and promote it religiously with an effort to maintain best practice.

b) Do a really thorough validation/sanity check cycle only at major release versions, or minor release versions?

c) Some other solution.

Im not saying I have an answer, I'm just trying to see what a pragmatic balance of effort to benefit is, and as the core development team what we should promote or support.

Worth a discussion @ThomasDowrick, @StephenThompson

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 26, 2018, 09:56

closed

thompson318 commented 4 years ago

In GitLab by @ThomasDowrick on Nov 26, 2018, 10:33

I think it is good to have requirements/specifications set out somewhere, especially as we're working with medical devices. This doesn't necessarily have to be stored in each repository, or be a formal software specification, it could be that the list of requirements (Data Capture/Algortitms/Viz etc.) we currently have on the SNAPPY page is sufficient, provided we update it as we progress.

Even with the best will in the world, keeping software documentation in two places up to date is going to be tricky, so I lean towards good API documetation and clear test/function/class names, which falls under option b) above I would say.

thompson318 commented 4 years ago

In GitLab by @StephenThompson on Nov 27, 2018, 09:23

To me the point of requirements.rst is that it might encourage some people to follow best practice, which is to think (and record what you think) before you code. To me it doesn't matter whether requirements are subsequently documented there or in the tests and/or the function code. If we were producing production ready medical device code under a QMS, that would have to more formalised, but at the momemt we're not and I don't think we have the resources to do that. If I was reviewing someone else's code (as in b above) I would want to see that the functions had meaningful documentation, and the tests made sense, before worrying about anything written (or not) in requirements.rst.

thompson318 commented 4 years ago

In GitLab by @MattClarkson on Nov 27, 2018, 12:36

OK, understood. Lets aim to encourage people to write down requirements first, before hacking away. Then if ultimately, the API and unit tests are sufficient, then thats fine. And maybe at specific releases people could cross reference if necessary.