Skip to content

chore: add reusable plugin workflow#1

Merged
Fdawgs merged 8 commits intofastify:mainfrom
Fdawgs:chore/initial-release
Dec 22, 2021
Merged

chore: add reusable plugin workflow#1
Fdawgs merged 8 commits intofastify:mainfrom
Fdawgs:chore/initial-release

Conversation

@Fdawgs
Copy link
Copy Markdown
Member

@Fdawgs Fdawgs commented Dec 13, 2021

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

@Fdawgs Fdawgs self-assigned this Dec 13, 2021
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@climba03003
Copy link
Copy Markdown
Member

Should we remove node@10 and wait until fastify@4 for the reusable workflow integration?

README.md Outdated
- '*.md'

jobs:
call-reuseable-workflow:
Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

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?

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member Author

@Fdawgs Fdawgs Dec 20, 2021

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 and steps at the same time in a job
  • You cannot define both uses and services at the same time in a job

Copy link
Copy Markdown
Member Author

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.

@Fdawgs Fdawgs requested a review from Eomm December 14, 2021 11:25
Copy link
Copy Markdown
Member

@Eomm Eomm left a 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:
Copy link
Copy Markdown
Member

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 <behemoth89@gmail.com>
@Fdawgs
Copy link
Copy Markdown
Member Author

Fdawgs commented Dec 18, 2021

Would you mind using two-space tag instead of 4 spaces on the yml files?

The 4 space style is due to me using Prettier for a lot of my stuff.
Standard, which seems to be the main style in the Fastify org, doesn't seem to support yml formatting.

I don't know if we need to decide on a standard format for yml and other file types?

@Eomm
Copy link
Copy Markdown
Member

Eomm commented Dec 18, 2021

Yes, standard is used to check js files.
I don't think that we need a yml 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

@Fdawgs
Copy link
Copy Markdown
Member Author

Fdawgs commented Dec 18, 2021

Yes, standard is used to check js files. I don't think that we need a yml 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.

Copy link
Copy Markdown
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

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

LGTM.

@Fdawgs
Copy link
Copy Markdown
Member Author

Fdawgs commented Dec 20, 2021

Yes, standard is used to check js files. I don't think that we need a yml 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.

Resolved by bc2a4f6

Copy link
Copy Markdown
Member

@Eomm Eomm left a 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

@Fdawgs Fdawgs merged commit 4658b2d into fastify:main Dec 22, 2021
@Fdawgs Fdawgs deleted the chore/initial-release branch December 22, 2021 12:19
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.

4 participants