Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Publishing/Open Source guidance too strict #297

Open
adam-carruthers opened this issue May 16, 2023 · 2 comments
Open

Publishing/Open Source guidance too strict #297

adam-carruthers opened this issue May 16, 2023 · 2 comments

Comments

@adam-carruthers
Copy link

adam-carruthers commented May 16, 2023

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
Copy link
Contributor

regularfry commented Jul 6, 2023

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 nhs-england-tools/repository-template#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.

github-merge-queue bot pushed a commit to nhs-england-tools/repository-template that referenced this issue Jul 20, 2023
<!-- markdownlint-disable-next-line first-line-heading -->
## Description

This PR adds `make` tasks for running tests.

## Context

[This
issue](NHSDigital/software-engineering-quality-framework#297)
says that we're out of alignment with wider policy in the sense that
we're applying stricter rules than government policy requires.

There are two ways to solve this: we can either loosen our rules, or
make them easier to comply with. From the point of view of the test
statuses we require in the quality framework
[here](https://github.com/NHSDigital/software-engineering-quality-framework/blob/main/quality-checks.md?plain=1#L66-L86),
this does the latter.

Specifically it allows the user of the template to put scripts into
`scripts/test` for running the various types of test required by the
`software-engineering-quality-framework` which, until they are
implemented, will output a warning to the console (but do not fail).

## Type of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply. -->

- [ ] Refactoring (non-breaking change)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would change existing
functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply. -->

- [ ] I am familiar with the [contributing
guidelines](../docs/CONTRIBUTING.md)
- [ ] I have followed the code style of the project
- [ ] I have added tests to cover my changes
- [ ] I have updated the documentation accordingly
- [ ] This PR is a result of pair or mob programming

---

## Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others
privacy, we kindly ask you to NOT including [PII (Personal Identifiable
Information) / PID (Personal Identifiable
Data)](https://digital.nhs.uk/data-and-information/keeping-data-safe-and-benefitting-the-public)
or any other sensitive data in this PR (Pull Request) and the codebase
changes. We will remove any PR that do contain any sensitive
information. We really appreciate your cooperation in this matter.

- [ ] I confirm that neither PII/PID nor sensitive data are included in
this PR and the codebase changes.

---------

Signed-off-by: regularfry <alex@blackkettle.org>
@adam-carruthers
Copy link
Author

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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants