-
Notifications
You must be signed in to change notification settings - Fork 145
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
Conversation
@@ -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 --> |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it !
There was a problem hiding this 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.
Yes, I'm still planning to document all this in written form #1038 |
I'd need to test this (and I will), but according to the docs (sentence after first paragraph - doesn't have a |
Merging this as Florian already approved the previous iteration and the only difference is a word in the repo name of a workflow |
@ijemmy I have tested your suggestion in a fork, by defining a dummy workflow at top level that calls another in a subfolder: |
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:
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:
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: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:
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:
Example of "On PR Merge" workflow - when a PR is still open:
Example of "On PR Merge" workflow - when a PR is actually merged:
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
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.