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

Improve CI #387

Merged
merged 9 commits into from
Aug 15, 2024
Merged

Improve CI #387

merged 9 commits into from
Aug 15, 2024

Conversation

wilwade
Copy link
Contributor

@wilwade wilwade commented Aug 14, 2024

Problems Fixed

  • GitHub wasn't able to require CI statuses pass for matrixes
  • Not everything was being checked
  • Docker Hub was hitting 429 api limits
  • Common changes were not triggering all the tests

To Revert

If statements for skipping when not needed

Getting the matrix working... maybe

Matrix doesn't work in job level if statements

Applying the if statement to every step

Caching docker layers

Trigger the build of the api specs as part of the build test

Try switching registries

Cleanup
@@ -9,71 +9,125 @@ on:
- main

jobs:
determine-service-matrix:
service-matrix:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rename so the lines don't get so long below

changes: ${{ steps.determine-matrix.outputs.changes }}
# IF you add a new filter it ALSO needs to be here
services: >-
["account", "graph", "content-publishing", "content-watcher"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to find a way not to have to do this, but no luck. This was still the best way.

working-directory: services/${{ matrix.service }}
# List all the licenses and error out if it is not one of the supported licenses
run: npx license-report --fields=name --fields=licenseType | jq 'map(select(.licenseType | IN("MIT", "Apache-2.0", "ISC", "BSD-3-Clause", "BSD-2-Clause", "(Apache-2.0 AND MIT)", "Apache-2.0 OR MIT") | not)) | if length == 0 then halt else halt_error(1) end'

- name: Build OpenAPI spec
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Part of the release that broke on v1.0, so this is to keep it from breaking.

with:
config-inline: |
[registry."ghcr.io"]
- name: Cache Docker layers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really sure this is worth doing, but I did it.

@@ -9,84 +9,103 @@ on:
- main

jobs:
determine-service-matrix:
service-matrix:
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 is the same set of changes as build.yml limited to the matrix changes and no additional jobs

echo "SERVICE=${SERVICE}" >> $GITHUB_OUTPUT
echo "SERVICE_MATRIX=${SERVICE_MATRIX}" >> $GITHUB_OUTPUT
echo "valid=${valid}" >> $GITHUB_OUTPUT
echo "test_run=${test_run}" >> $GITHUB_OUTPUT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update to the new version of this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

formatting

@@ -15,7 +15,7 @@ process.env.PROVIDER_ACCOUNT_SEED_PHRASE =
'offer debate skin describe light badge fish turtle actual inject struggle border';
process.env.PROVIDER_ID = '0';
process.env.WEBHOOK_BASE_URL = 'http://127.0.0.1';
process.env.CAPACITY_LIMIT = '{"type":"amount","value":"1"}';
process.env.CAPACITY_LIMIT = '{"type":"amount","value":"80"}';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are updates to fix the broken swagger builds

@wilwade wilwade marked this pull request as ready for review August 14, 2024 21:39
Copy link
Contributor

@mattheworris mattheworris left a comment

Choose a reason for hiding this comment

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

  • Read through changes
    Looks good, lgtm!

@wilwade wilwade merged commit 6080700 into main Aug 15, 2024
11 checks passed
@wilwade wilwade deleted the chore/improve-ci branch August 15, 2024 00:44
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