Skip to content

tools versions & cicd publish-stage & codespace devcontainer #105

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.

Sign up for GitHub

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

Closed
RossBugginsNHS wants to merge 13 commits into from
Closed

tools versions & cicd publish-stage & codespace devcontainer #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
302f7a2
bypass acceptance
RossBugginsNHS Nov 12, 2025
6bed1a7
publish-stage to be dependant on metadata and build, not acceptance.
RossBugginsNHS Nov 12, 2025
5e701f9
Set default root tools to node 22.11.0
RossBugginsNHS Nov 12, 2025
3c7f54b
Remove docker section from cloud events tools. This file could be rem…
RossBugginsNHS Nov 12, 2025
b7d312f
bump event catalog version and add readme to src folder.
RossBugginsNHS Nov 12, 2025
cb6a128
readme added.
RossBugginsNHS Nov 12, 2025
6c7de85
move dev container to root.
RossBugginsNHS Nov 12, 2025
606bbe8
Add dev container that supports github codespaces development.
RossBugginsNHS Nov 13, 2025
88f48b4
Updated _install-dependencies to recursively install all tool-version…
RossBugginsNHS Nov 14, 2025
43f72aa
Updating tools versions to only have what is required. Node24 is requ…
RossBugginsNHS Nov 14, 2025
586c522
Stop welcome screen being shown, speed up load.
RossBugginsNHS Nov 14, 2025
08e818b
Stop terminal auto opening, to make it clear to user that initial con…
RossBugginsNHS Nov 14, 2025
29b7644
Automatic package lock updates.
RossBugginsNHS Nov 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"containerEnv": {
"GITHUBMONITOR": "false",
"MAKECONFIG": "true",
"SHOWWELCOME": "false",
"UPDATEFROMTEMPLATE": "false"
},
"image": "ghcr.io/nhsdigital/nhs-notify-devcontainer-loaded-codespaces:1.0.19",
"name": "Digital Letters Codespaces"
}
6 changes: 3 additions & 3 deletions ...ify-devcontainer-loaded/devcontainer.json → .devcontainer/local-dev/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
"containerEnv": {
"GITHUBMONITOR": "false",
"MAKECONFIG": "true",
"SHOWWELCOME": "true",
"SHOWWELCOME": "false",
"UPDATEFROMTEMPLATE": "false"
},
"image": "ghcr.io/nhsdigital/nhs-notify-devcontainer-loaded:1.0.17",
"name": "Notify Loaded 1.0.17",
"image": "ghcr.io/nhsdigital/nhs-notify-devcontainer-loaded:1.0.19",
"name": "Digital Letters Local Dev",
"postStartCommand": "mkdir -p ~/.gnupg && echo '## 1-day timeout' > ~/.gnupg/gpg-agent.conf && echo 'default-cache-ttl 86400' >> ~/.gnupg/gpg-agent.conf && echo 'max-cache-ttl 86400' >> ~/.gnupg/gpg-agent.conf && gpg-connect-agent reloadagent /bye 2>/dev/null || true"
}
3 changes: 1 addition & 2 deletions .github/workflows/cicd-1-pull-request.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,7 @@ jobs:
APP_CLIENT_ID: ${{ secrets.APP_CLIENT_ID }}
publish-stage: # Recommended maximum execution time is 10 minutes
Copy link
Contributor

@Ian-Hodges Ian-Hodges Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be better moved to pr_closed? There it will only run on main and after the acceptance tests have run.

Copy link
Contributor Author

@RossBugginsNHS RossBugginsNHS Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reason I've kept it here, is so i can push direct to main on a fork, and have this go through. otherwise on a fork when testing CICD to GHP, you would have to do your own pr to yourself.

Copy link
Contributor

@Ian-Hodges Ian-Hodges Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be making compromises to support forks.

Copy link
Contributor Author

@RossBugginsNHS RossBugginsNHS Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any suggestion on how to support forks?

Copy link
Contributor

@Ian-Hodges Ian-Hodges Nov 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this isolated example you can amend the workflow on the forked repo.
It's not supporting forks, no. If you want full fork support I think it would be best to move it out to its own repo.

Copy link
Contributor Author

@RossBugginsNHS RossBugginsNHS Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this is a wider conversation to fully move the publish stage out of here. Probably need to look at how our repository template (https://github.com/NHSDigital/nhs-notify-repository-template/tree/main/.github/workflows) is different from the NHSE template (https://github.com/nhs-england-tools/repository-template/tree/main/.github/workflows), in that we don't have the separate publish workflow, and that to get around that for this to be able to publish its just in https://github.com/NHSDigital/nhs-notify-digital-letters/blob/main/.github/workflows/cicd-1-pull-request.yaml instead.

Copy link
Contributor Author

@RossBugginsNHS RossBugginsNHS Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to suggest that removing this at present is a breaking changes for a fork, so to leave it in. Then will look separately at bringing in the workflow "2 publish" and look at fully moving the publish stage out of this workflow?

Copy link
Contributor

@Ian-Hodges Ian-Hodges Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to leave this as-is while we decide on the best place for it.

name: "Publish stage"
needs: [metadata, acceptance-stage] #PUT THIS BACK WHEN ACCEPTANCE STAGE IS ENABLED
#needs: [metadata, build-stage] BYPASSING ACCEPTANCE STAGE
needs: [metadata, build-stage]
uses: ./.github/workflows/stage-5-publish.yaml
if: (github.event_name == 'push' && github.ref == 'refs/heads/main')
with:
Expand Down
4 changes: 2 additions & 2 deletions .tool-versions
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
act 0.2.64
gitleaks 8.24.0
jq 1.6
nodejs 24.10.0
nodejs 22.11.0
pre-commit 3.6.0
python 3.13.2
python 3.14.0
terraform 1.10.1
terraform-docs 0.19.0
trivy 0.61.0
Expand Down
2 changes: 2 additions & 0 deletions docs/.tool-versions
View file
Open in desktop
Copy link
Contributor

@Ian-Hodges Ian-Hodges Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of all these .tool-versions files. Do these packages need node 24?

Copy link
Contributor Author

@RossBugginsNHS RossBugginsNHS Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure if they will play ball with 22. I do think this is the point of tools versions though so can do monorepo support for tools versioning?

Copy link
Contributor

@Ian-Hodges Ian-Hodges Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it works well with npm workspaces and make where most commands are ran from the root directory. If you are keeping them, I presume you don't need all those tools i.e. terraform stuff?

Copy link
Contributor Author

@RossBugginsNHS RossBugginsNHS Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of the make uses make -C so it runs child ones from their directory. Have gone through and removed all redundant lines. Was a good spot though as the template doesn't proactively install child tools versions - something that we've missed as there is an existing tool-versions nested in there. Have done a separate pr to backport changes - see change to init.mk

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
nodejs 24.11.1
python 3.14.0
Loading