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

Update guidance: test "job" is now CI "workflow" #4340

Merged
merged 1 commit into from
Dec 27, 2023

Conversation

ChrisBAshton
Copy link
Contributor

@ChrisBAshton ChrisBAshton commented Dec 11, 2023

As described in the documentation change, we no longer define
test jobs, and now define CI workflows, which work slightly
differently insofar as they're made up of several parallel jobs.

This commit makes a few other updates, including:

  • Remove reference to small steps within a CI workflow:
    We've actually been heading in the opposite direction: lots of
    small steps defined within the CI workflow. This is partly because
    we're now making use of a lot of shared workflows, so what we lose
    in ease of local development, we gain in cross-GOV.UK consistency.
  • Remove old example code
    Looking at the Content Publisher GitHub Actions, we no longer have
    the complexity referred to in these docs. I've deleted the docs
    that are no longer relevant.
    I've kept the instructions for testing Ruby gems, as that is still
    being applied.
  • Move "previous recommendation" section further up:
    Moving it closer to where we're justifying our choices for "when
    the CI workflow should run".
    Also creates a new subsection for running CI on demand.
  • Softening the language around omitting names for jobs and steps.
    In practice, a great many of our repos needlessly define job and
    step names that override the job/step key, but there is no harm
    in doing so.

ChrisBAshton added a commit to alphagov/govuk-dependabot-merger that referenced this pull request Dec 11, 2023
We've moved from looking for a job called 'test' or 'test-ruby',
to looking for a workflow called 'CI'.
See alphagov/govuk-developer-docs#4340
@ChrisBAshton ChrisBAshton force-pushed the update-github-actions-guidance branch from 29fe354 to 59ccc69 Compare December 27, 2023 14:23
As described in the documentation change, we no longer define
`test` jobs, and now define `CI` workflows, which work slightly
differently insofar as they're made up of several parallel jobs.

This commit makes a few other updates, including:

- Remove reference to small steps within a CI workflow:
  We've actually been heading in the opposite direction: lots of
  small steps defined within the CI workflow. This is partly because
  we're now making use of a lot of shared workflows, so what we lose
  in ease of local development, we gain in cross-GOV.UK consistency.
- Remove old example code
  Looking at the Content Publisher GitHub Actions, we no longer have
  the complexity referred to in these docs. I've deleted the docs
  that are no longer relevant.
  I've kept the instructions for testing Ruby gems, as that is still
  being applied.
- Move "previous recommendation" section further up:
  Moving it closer to where we're justifying our choices for "when
  the CI workflow should run".
  Also creates a new subsection for running CI on demand.
- Softening the language around omitting names for jobs and steps.
  In practice, a great many of our repos needlessly define job and
  step names that override the job/step key, but there is no harm
  in doing so.
@ChrisBAshton ChrisBAshton force-pushed the update-github-actions-guidance branch from 59ccc69 to eafbef9 Compare December 27, 2023 14:28
@ChrisBAshton ChrisBAshton changed the title Update GitHub actions guidance Update guidance: test "job" is now CI "workflow" Dec 27, 2023
@ChrisBAshton ChrisBAshton marked this pull request as ready for review December 27, 2023 14:31
Copy link
Contributor

@AgaDufrat AgaDufrat left a comment

Choose a reason for hiding this comment

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

What do you think about bumping the “Reuse workflows where possible’ section up to the top of this doc? It has fairly general content, relevant for the later, more specific parts of this manual. We also mention it in the “Name of CI workflow and jobs”.
When thinking about dev process for these, we would need to define the reusable workflow first to then use in the CI, so seems like a logical order. But I don't have strong feelings about it, so good to merge as it is ;)

@ChrisBAshton ChrisBAshton merged commit 1d682fd into main Dec 27, 2023
5 checks passed
@ChrisBAshton ChrisBAshton deleted the update-github-actions-guidance branch December 27, 2023 16:06
@ChrisBAshton
Copy link
Contributor Author

Thanks @AgaDufrat - I like the suggestion, but also like the top-down approach of "this is the file", "this is how it's structured", and then eventually "this is where we define our reusable workflows"...

But I think a future restructure would be good, making it easier to dive in and find what you need (and can probably be done in a far less wordy way than the current iteration). 👍

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.

2 participants