-
Notifications
You must be signed in to change notification settings - Fork 8
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
chore: add reusable plugin workflow #1
Conversation
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.
lgtm
Should we remove node@10 and wait until fastify@4 for the reusable workflow integration? |
README.md
Outdated
- '*.md' | ||
|
||
jobs: | ||
call-reuseable-workflow: |
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.
We should add an example on how to use it with an external source, like mongodb
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.
Do we know if this is possible to do?
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.
Not 100% on what you mean @Eomm , sorry!
Do you mean using services like what is done in some of the Fastify repos such as fastify-postgres?
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.
Yes, exactly!
I don't understand if this is working reading the docs
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.
From reading the docs, I have no idea either!
I imagine it would look like the workflow below, but can't confirm it would work.
I can open a test PR on fastify-postgres to see if it would work with my fork?
name: CI
on:
push:
paths-ignore:
- 'docs/**'
- '*.md'
pull_request:
paths-ignore:
- 'docs/**'
- '*.md'
jobs:
call-reuseable-workflow:
runs-on: ubuntu-latest
services:
postgres:
image: postgres:11-alpine
env:
POSTGRES_USER: postgres
POSTGRES_DB: postgres
POSTGRES_PASSWORD: postgres
ports:
- '5432:5432'
options: >-
--health-cmd pg_isready --health-interval 10s --health-timeout 5s
--health-retries 5
steps:
- uses: fastify/workflows/.github/workflows/plugins-ci.yml@v1
with:
generate-coverage: true
Edit: Unable to get it to work with my attempts.
From the errors returned:
- Reusable workflows should be referenced at the top-level
jobs.*.uses
key, not within steps - You cannot define both
uses
andsteps
at the same time in a job - You cannot define both
uses
andservices
at the same time in a job
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.
Think we'd have to create variations of the workflow to support use of services.
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.
Would you mind using two-space tag instead of 4 spaces on the yml files?
README.md
Outdated
- '*.md' | ||
|
||
jobs: | ||
call-reuseable-workflow: |
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.
Do we know if this is possible to do?
Co-authored-by: Manuel Spigolon <[email protected]>
The 4 space style is due to me using Prettier for a lot of my stuff. I don't know if we need to decide on a standard format for yml and other file types? |
Yes, standard is used to check js files. I'm suggesting the 2 spaces because it fits better on mobile gh app when there is no pc at disposal, and to copy-cat the standard style as well |
Cool beans, will take a pass at it on Monday when back in work. |
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.
LGTM.
Resolved by bc2a4f6 |
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.
LGTM
Just one note: before cutting a v1
release I would test how the services
keyword works as some most used plugins need to set up this feature
Created a basic reusable GitHub Action workflow that can be used across all of the plugins in the Fastify org.
Also integrates changes to coveralls actions as mentioned in fastify/fastify#3328, and the latest fastify/github-action-merge-dependabot version.
Checklist
and the Code of conduct