search

pact-foundation / pact-php

PHP version of Pact. Enables consumer driven contract testing, providing a mock service and DSL for the consumer project, and interaction playback and verification for the service provider project
Apache License 2.0
273 stars 92 forks source link

Maintenance Checklist #254

Closed YOU54F closed 1 month ago

YOU54F commented 2 years ago

Hey @cfmack,

Just beginning to set up maintenance checklists against the various repos, and since I am here probably a good place to start

I'm happy to pick most of these up over time, but just jotting them somewhere for now

YOU54F commented 2 years ago

We get duplicate builds on PR's because of

https://github.com/pact-foundation/pact-php/blob/f193e3c82fdd8a1cfe0dea32900f237a32a70019/.github/workflows/build.yml#L4-L6

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target

do we need both? if we need both, if they both trigger at the same time, should it cancel one?

one build fails, one passes

https://github.com/pact-foundation/pact-php/actions/runs/2396687319 https://github.com/pact-foundation/pact-php/actions/runs/2396687271

Lewiscowles1986 commented 2 years ago

I've never seen pull_request_target used before. I personally prefer pull_request. But I think there may be extra yaml that can prevent duplicate runs as well as removing. What do others think?

Logic should be that without conflicts, master should broadly always pass; allowing push to go away too.

YOU54F commented 2 years ago

pull_request_target sounds scary, there is a big wraning

it's weird it says

but then also states

This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does. This prevents execution of unsafe code from the head of the pull request that could alter your repository or steal any secrets you use in your workflow. This event allows your workflow to do things like label or comment on pull requests from forks. Avoid using this event if you need to build or run code from the pull request.

Warning: For workflows that are triggered by the pull_request_target event, the GITHUB_TOKEN is granted read/write repository permission unless the permissions key is specified and the workflow can access secrets, even when it is triggered from a fork. Although the workflow runs in the context of the base of the pull request, you should make sure that you do not check out, build, or run untrusted code from the pull request with this event. Additionally, any caches share the same scope as the base branch. To help prevent cache poisoning, you should not save the cache if there is a possibility that the cache contents were altered. For more information, see "Keeping your GitHub Actions and workflows secure: Preventing pwn requests" on the GitHub Security Lab website.

I've used

on:
  push:
  pull_request:
    branches: [main]

which means CI always runs on any push, and on pull_request only to main

https://github.com/pactflow/pact-msw-adapter/blob/main/.github/workflows/build-and-test.yml

There is a few improvements I want to make to these workflows, a-lot of my train of thought was actually documented in here https://ncorti.com/blog/howto-github-actions-build-matrix but that doesn't the answer on the ideal workflow triggers

YOU54F commented 1 year ago

Developing Guide :- https://github.com/pact-foundation/pact-php/blob/ffi/DEVELOPING.md

Release notes are generated when creating a release, see releasing notes, we don't create a distinct CHANGELOG file

YOU54F commented 1 month ago

Going to close this off now, I think we are in a pretty happy place.

For anything else that people want actioned, we can raise as separate tickets