-
Notifications
You must be signed in to change notification settings - Fork 20
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #105 from ukhsa-collaboration/dev
Silent PR to Master to check updated documentation
- Loading branch information
Showing
123 changed files
with
16,338 additions
and
5,512 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
## Description of the PR | ||
|
||
<!-- Add a description of your changes below this line --> | ||
Replace this text with a description of the pull request including the issue number. | ||
|
||
## Checks | ||
|
||
This is an itemised checklist for the QA process within UKHSA and represents the bare minimum a QA should be. | ||
|
||
Full instructions on reviewing work can be found at Confluence on the [ONS QA of code guidance](https://best-practice-and-impact.github.io/qa-of-code-guidance/intro.html) page. | ||
|
||
**To the reviewer:** Check the boxes once you have completed the checks below. | ||
|
||
- [ ] It runs | ||
- Can you get the code run to completion in a new instance and from top to bottom in the case of notebooks, or in a new R session? | ||
- Can original analysis results be accurately & easily reproduced from the code? | ||
- [] tests pass | ||
- [] CI is successful | ||
This is a basic form of Smoke Testing | ||
- [ ] Data and security | ||
- Use nbstripout to prevent Jupyter notebook output being committed to git repositories | ||
- Files containing individual user's secret files and config files are not in repo, however examples of these files and setup instructions are included in the repo. | ||
- Secrets include s3 bucket names, login credentials, and organisation information. These can be handled using secrets.yml | ||
- If you are unsure whether an item should be secret please discuss with repo owner | ||
- The changes do not include unreleased policy or official information. | ||
- [ ] Sensible | ||
- Does the code execute the task accurately? This is a subjective challenge. | ||
- Does the code do what the comments and readme say it does\*? | ||
- Is the code robust enough to handle missing or challenging data? | ||
- [ ] Documentation | ||
- The purpose of the code is clearly defined, whether in a markdown chunk at the top of a notebook or in a README | ||
- Assumptions of the analysis & input data are clearly displayed to the reader, whether in a markdown chunk at the top of a notebook or in a README | ||
- Comments are included in the code so the reader can follow why the code behaves in the way it does | ||
- Teams with high quality documentation are better able to implement technical practices more readily and perform better as a whole (DORA, 2021). | ||
- Is the code written in a standard way? (In a hurry this may be a nice to have, but if at all possible, this standard from the beginning can cut long term costs dramatically) | ||
- Code is modular, storing functions & classes in the src and being imported into a notebook or script | ||
- Projects should be based on the UKHSA repo template developed to work with cookiecutter | ||
- Variable, function & module names should be intuitive to the reader | ||
- For example, intuitive names include df_geo_lookup & non-intuitive names include foobar | ||
- Common and useful checks for coding we use broadly across UKHSA include: | ||
- Rstyler | ||
- lintr | ||
- black | ||
- flake8 | ||
- [ ] Pair coding review completed (optional, but highly recommended for QA in a hurry) | ||
- Pair programming is a way of working and reviewing that can result in the same standard of work being completed 40%-50% faster (Williams et al., 2000, Nosek, 1998) and is better than solo programming for tasks involving efficient knowledge transferring and for working on highly connected systems (Dawande et al., 2008). | ||
- Have the assignee and reviewer been on a video call or in person together during the code development in a line by line writing and review process? | ||
|
||
\* If the comments or readme do not have enough information, this check fails. | ||
|
||
## How to QA this PR | ||
|
||
Before accepting the PR and merging to `main` or `master`, please follow these steps on a terminal in your development instance: | ||
|
||
- `git status` check what branch you are on and if you have any uncommitted changes. | ||
- Handle the work on your current branch: | ||
- `git commit` if you would like to keep the changes you have made on your current branch. | ||
- `git stash` if you do not want to keep these changes, although you can recover these later. | ||
- `git checkout main` or `git checkout master` to go to the branch that will be merged to. | ||
- `git pull origin main` or `git pull origin master` fetches the most up to date git info on this branch. | ||
- `git branch -a` lists all the available branches. | ||
- `git checkout BRANCHNAME-FOR-PR` moves you into the branch to QA. | ||
- `git pull origin BRANCHNAME-FOR-PR` ensures you have the most recent changes for the PR. | ||
- Run the notebooks or code. | ||
- Run `pre-commit`. This runs some automated checks to check the code is well formatted and does not contain data. | ||
- Are there any small changes that can be made to get it to run? | ||
- Yes: make annotations on github and notify code creator to correct them | ||
- No, it runs: done! | ||
- No, and it looks like it would need a lot of work: note major points in Github PR chat and discuss with author. | ||
- Carefully read through the code or documentation and make sure it makes sense. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
name: build-book | ||
|
||
# only run when specific files change | ||
# only run on install-fix-docs branch | ||
# TODO change this to dev/main branch once integrated | ||
|
||
on: | ||
push: | ||
branches: | ||
- master | ||
|
||
jobs: | ||
build-book: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 | ||
|
||
- uses: actions/cache@v3 | ||
with: | ||
path: ~/.cache/pip | ||
key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }} | ||
restore-keys: | | ||
${{ runner.os }}-pip- | ||
steps: | ||
- uses: actions/checkout@v3 | ||
|
||
- name: Setup Graphviz | ||
uses: ts-graphviz/setup-graphviz@v1 | ||
|
||
# install python | ||
- name: Set up Python 3.8 | ||
uses: actions/setup-python@v4 | ||
with: | ||
python-version: 3.8 | ||
|
||
# install dependencies | ||
- name: Install dependencies | ||
run: | | ||
python -m pip install --upgrade pip | ||
pip install -r requirements.txt | ||
pip install -r docs/requirements.txt | ||
# set up pygom | ||
- name: Build and install pygom | ||
run: | | ||
python setup.py build | ||
python setup.py install | ||
# build the book | ||
# TODO check which flags are needed, -W | ||
- name: Build the book | ||
run: | | ||
jupyter-book build --all -v docs/ | ||
# deploy book to github-pages | ||
- name: GitHub Pages | ||
uses: peaceiris/[email protected] | ||
with: | ||
github_token: ${{ secrets.GITHUB_TOKEN }} | ||
publish_dir: docs/_build/html | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
# Contributor guidance | ||
|
||
By contributing to PyGOM, you acknowledge that you are assigning copyright to us so we can continue to maintain package. PyGOM is licensed under (GPL2)[https://raw.githubusercontent.com/PublicHealthEngland/pygom/master/LICENSE.txt]. | ||
|
||
## suggested information for raising issues | ||
- is it possible to produce a template for a new issue? | ||
|
||
## requirements for amendments | ||
- merge and pull requests should only be considered if the MR/PR explicitly states how the following has been addressed | ||
- unit tests | ||
- modified documentation | ||
- justification | ||
- successful execution of documentation (controlled by halting at errors in /docs/_config.yml, demonstrated by successful action) | ||
- #TODO is there a reasonable limit on the code quantity that is submitted per review? (e.g. 200 lines) to ensure a suitable review can be completed | ||
|
||
## workflow for incorporating merge and pull requests | ||
- dev and master are protected and require a code review by the project owner (or designated reviewer) | ||
- dev should contain all working amendments and collated before the next versioned merge request to master | ||
- only a merge request should be made to master from dev from the UKHSA repo | ||
- forked repo pull requests should be made into UKHSA dev branch (or others where appropriate) | ||
- master branch is for versioned releases, which should be tagged and a summary of alterations made to README.md | ||
- merge and pull requests to be made to dev, where builds and actions must run successfully before being incorporated | ||
- if closing a ticket, if possible, include the commit reference which addresses the issue | ||
- code reviewer should use the template to work through requirements (e.g. confirming tests have been added, documentation is appropriate, added to contributor file) | ||
|
||
## adding to the documentation how to add to the jupyterbook; | ||
The documentation directory (docs)[docs/] contains folders for each filetype used to build the documentation. Folders that exist under these are associated with sections (defined by [docs/_toc.yml]()). The basic steps for adding pages to the Jupyter Book are here, and for more detailed configuration see comments in the config file)[docs/_config.yml], Jupyter Book documentation, or Sphinx documentation. | ||
- save your ipynb, md, or rst file to the appropriate folder | ||
- update the (table of contents file)[docs/_toc.yml] so that the file is incorporated | ||
- from the root of the repository run `jupyter-book build docs/' to build the html files | ||
- check for build errors or warnings, and view your additions in `docs/_build/html` | ||
|
||
## acknowledgements from contributors | ||
- what counts as a contribution? | ||
- ticket? pr/mr? | ||
- how are contributors acknowledged? | ||
- contributor md file? | ||
- who adds the name to the contributor? suggest code reviewer on approval of MR/PR | ||
- #TODO provide citable repo link? |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,4 +10,6 @@ dependencies: | |
- sympy | ||
- graphviz | ||
- cython | ||
# - pip: [] | ||
- pip | ||
-pip: | ||
- -r requirements.txt |
Oops, something went wrong.