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

fix(layers): release process + remove duplicate code #1052

Merged
merged 3 commits into from
Aug 12, 2022

Conversation

dreamorosi
Copy link
Contributor

@dreamorosi dreamorosi commented Aug 11, 2022

Description of your changes

This PR tackles two separate but related issues: #1047 and #1053. All changes are related to the workflows that run as a result of a PR, a merge, or a release.

In an effort to try to minimise issues related to workflows this PR tries to remove duplicate code across workflows by creating a reusable and separate workflow around two flows:

  • Running unit tests on all utils, examples, and layer-publisher
  • Publishing docs (both main docs & API)

These generalised workflows can now be called by another workflow so that the amount of repeated code is reduced.

For example, before this change, the same code for running the unit tests was repeated in the workflows that run:

  • On merge
  • On a PR update
  • On a release

Every time a change was needed or a new directory was added us maintainers had to go in each file and replicate the changes. This was needlessly time consuming and error prone, especially this last point is proved by the uptick of issues generated by the introduction of a new folder (layer-publisher).

With this change, the logic needed to run the tests has been extracted in a separate workflow that is now found at .github/workflows/reusable-run-unit-tests.yml. This workflow can be called by another workflow this way:

run-unit-tests:
    uses: ./.github/workflows/reusable-run-unit-tests.yml

This way, the three workflows mentioned above will have only these two lines, and any change required to the logic will happen in a single place.

The steps needed to publish the docs also followed a similar pattern. The logic was replicated across two workflows that run:

  • On merge
  • On a release

In this case, the steps had slight differences related to the fact that we want to publish the docs under two different paths based on the stage.

This PR creates a reusable workflow called .github/workflows/reusable-publish-docs.yml that accepts some inputs that will help the workflow to publish on the right path based on the values passed.

Note: this PR also sneaks in a minor wording change in the PR_TEMPLATE file that was causing some workflows that are in charge of labelling issues to fail.

How to verify this change

I have ran the various the workflows in a fork but please note that some of them fail at later stages because the token needed to publish to NPM is not present in the fork.

Example of "Make Release" workflow:
image

Example of "On PR Merge" workflow - when a PR is still open:
image

Example of "On PR Merge" workflow - when a PR is actually merged:
image

To see an example of the workflow that runs when a PR is updated, see the checks below.

Related issues, RFCs

Issue number: #1047
Issue number: #1053

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • The PR title follows the conventional commit semantics

Breaking change checklist

N/A


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dreamorosi dreamorosi added the fix label Aug 11, 2022
@dreamorosi dreamorosi added this to the Lambda layer milestone Aug 11, 2022
@dreamorosi dreamorosi self-assigned this Aug 11, 2022
@dreamorosi dreamorosi added this to Pull Requests - Work in progress in Pull Requests via automation Aug 11, 2022
@dreamorosi dreamorosi linked an issue Aug 11, 2022 that may be closed by this pull request
@github-actions github-actions bot added the bug Something isn't working label Aug 11, 2022
@@ -21,8 +21,8 @@

### Related issues, RFCs

<!--- Add here the number to the Github Issue or RFC that is related to this PR. -->
<!-- **Issue number:** #123 -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line was interpreted by the workflow as an actual issue number. The workflow was then trying to assign a label to it but since it doesn't exist it would fail.

@dreamorosi dreamorosi marked this pull request as draft August 11, 2022 17:48
@dreamorosi dreamorosi changed the title fix(layers): release process fix(layers): release process + remove duplicate code Aug 11, 2022
@dreamorosi dreamorosi marked this pull request as ready for review August 11, 2022 18:48
flochaz
flochaz previously approved these changes Aug 12, 2022
Copy link
Contributor

@flochaz flochaz left a comment

Choose a reason for hiding this comment

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

Love it !

Pull Requests automation moved this from Pull Requests - Work in progress to Pull Requests - Review needed Aug 12, 2022
Copy link
Contributor

@ijemmy ijemmy left a comment

Choose a reason for hiding this comment

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

Please double check the value in inputs.workflow_origin

(Optional)
Would it be possible to put reusable workflows in a reusable directory?

It might also worth going through this in our maintainer sync so everyone is aware, how we structure them + naming convention.

.github/workflows/reusable-publish-docs.yml Outdated Show resolved Hide resolved
@dreamorosi
Copy link
Contributor Author

It might also worth going through this in our maintainer sync so everyone is aware, how we structure them + naming convention.

Yes, I'm still planning to document all this in written form #1038

@dreamorosi
Copy link
Contributor Author

(Optional) Would it be possible to put reusable workflows in a reusable directory?

I'd need to test this (and I will), but according to the docs (sentence after first paragraph - doesn't have a /*) & this SO answer, GitHub expects the workflow to be only in the .github/workflows directory and subdirectories might not work.

@dreamorosi
Copy link
Contributor Author

Merging this as Florian already approved the previous iteration and the only difference is a word in the repo name of a workflow

@dreamorosi dreamorosi merged commit f653c06 into main Aug 12, 2022
Pull Requests automation moved this from Pull Requests - Review needed to Pull Requests - Merged or Closed Aug 12, 2022
@dreamorosi dreamorosi deleted the 1050-bug-layers-release-process-is-broken branch August 12, 2022 11:43
@dreamorosi
Copy link
Contributor Author

@ijemmy I have tested your suggestion in a fork, by defining a dummy workflow at top level that calls another in a subfolder:
image
it throws this error when pushed preventing to start the workshop:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Pull Requests
Pull Requests - Merged or Closed
Development

Successfully merging this pull request may close these issues.

Maintenance: release process is broken
3 participants