search

NHSDigital / software-engineering-quality-framework

🏎️ Shared best-practice guidance & tools to support software engineering teams
142 stars 35 forks source link

Publishing/Open Source guidance too strict #297

Open adam-carruthers opened 1 year ago

adam-carruthers commented 1 year ago

In the open source guidance in this framework it says that code must have unit tests, integration tests, API tests and more besides.

https://github.com/NHSDigital/software-engineering-quality-framework/blob/main/quality-checks.md?plain=1#L66-L86

This contradicts other guidance released by the NHS and in government:

The gist of these policies is that code should be open by default, and it should be open whether or not the code follows all the best practice (like having tests). Someone who wrote the Goldacre review said to me that code should be published even if it is bad. In fact, the policies even suggest coding in the open (from the start having your code be open source) as the ideal.

For this reason, it would be better if the quality framework changed some of its requirements from "required minimums" to "nice to haves" - or maybe it should even delete those requirements. This would mean this document aligns with what other government guidance suggests.

To be clear - I think you totally should have those tests, but that shouldn't be a requirement for open sourcing the code.

regularfry commented 1 year ago

I tend to agree with this, but I think the better position would be to support projects to get a placeholder structure in place from the start with as little effort as possible. To meet AMBER the tests don't actually have to do anything - just having a make test:unit task that spits an UNIMPLEMENTED warning to the console would satisfy it, as currently written (which may not be intentional, but that's what I see). The advantage of doing it that way is that we can work towards having a common interface across projects, incrementally; it's aligned with https://github.com/nhs-england-tools/repository-template/pull/76 and would be something I could PR into the Makefile...

The one place I personally don't think we should flex is the secret scanning, and I'm a little surprised that's AMBER rather than GREEN.

adam-carruthers commented 1 year ago

I still think that to say "you have to have tests to publish, but look we made it easy" is not the correct tack to take.

For instance, if a person is asking themselves "can I publish code in this repository we already have?" then they might think "no I can't" based on this guidance - and the template is not useful because this is a repository they already have.

Central government guidance from 2017 https://www.gov.uk/government/publications/open-source-guidance/when-code-should-be-open-or-closed suggests that person should definitely say "yes I can" and even "yes I should".