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

DAS-2043 - Add GitHub workflows for CI/CD. #1

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

owenlittlejohns
Copy link
Member

@owenlittlejohns owenlittlejohns commented Jan 3, 2024

Description

This PR adds CI/CD to this repository in the form of GitHub workflows:

  • Tests should be run for PRs (and every change to them).
  • A workflow will run the tests and then try to publish a new version of the Swath Projector Docker image when code is merged to the main branch containing a change to docker/service_version.txt.

The files in this PR were copied from the HOSS repository. The main thing I think with this PR is to check is that things now refer to the Swath Projector, not HOSS.

Jira Issue ID

DAS-2043.

Local Test Steps

Not really local testing...

  • This PR should trigger the unit tests automatically. They should pass.
  • I think Snyk should also run, as a webhook has been configured for it.

PR Acceptance Checklist

  • Jira ticket acceptance criteria met.
  • CHANGELOG.md updated to include high level summary of PR changes.
  • VERSION updated if publishing a release.
  • Tests added/updated and passing.
  • Documentation updated (if needed).

@owenlittlejohns
Copy link
Member Author

@lyonthefrog - it looks like another repository where you might not be already a maintainer. It might be worth updating your GitHub NAMS request to include a couple more of these repos. (We can chat about that in Slack)

But generally - here is the second part of the Swath Projector migration: adding the CI/CD. Part one was moving the code here, while preserving git commit history.

Copy link
Member

@flamingbear flamingbear left a comment

Choose a reason for hiding this comment

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

I only had one question and I'm sure the answer is obvious so I'll approve. Looks great.

@@ -0,0 +1,83 @@
# This workflow will run when changes are detected in the `main` branch, which
# must include an update to the `docker/service_version.txt` file. The workflow
# can also be manually triggered by a repository maintainer. This workflow will
Copy link
Member

Choose a reason for hiding this comment

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

If this workflow can be triggered by a maintainer, but it doesn't have a change to the service_version.txt file, will that overwrite a release?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that scenario would probably cause issues; for example part of the workflow is creating a git tag, and that would be a problem. So far these workflows have relied on people not trying to do manual releases. (But if you give someone a button, they will try and press it eventually)

The primary reason I added the manual release to these repositories was to enable the release of v1.0.0. I could definitely be convinced to remove the manual trigger in a subsequent PR.

@flamingbear
Copy link
Member

@lyonthefrog pssst here.

@lyonthefrog
Copy link
Collaborator

In the "All checks have passed" box in these comments, there's a "Run Python unit tests for pull requests...." check. Is this not what this PR is establishing, or is it something different?

@owenlittlejohns
Copy link
Member Author

In the "All checks have passed" box in these comments, there's a "Run Python unit tests for pull requests...." check. Is this not what this PR is establishing, or is it something different?

Yup - that's what we're looking for. One nice thing is that the workflows that are run in a PR are from the branch that you want to merge in, so it lets you know things are working nicely before you merge.

@owenlittlejohns owenlittlejohns merged commit b8bbe36 into main Jan 4, 2024
3 checks passed
@owenlittlejohns owenlittlejohns deleted the DAS-2043-add-CICD branch January 4, 2024 21:25
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

Successfully merging this pull request may close these issues.

3 participants