rsm-hcd / AndcultureCode.Cli

and-cli command-line tool to manage the development of software applications
https://andculture.com
Apache License 2.0
14 stars 15 forks source link

Github Actions - skip PR or merge builds from all-contributors bot #211

Closed brandongregoryscott closed 3 years ago

brandongregoryscott commented 3 years ago

Right now builds are triggered on all PRs, even builds from @all-contributors. While it isn't really hurting anything to run more builds, it's unnecessary and spams up our CI channel. The commits are tagged with [skip ci] but GA does not seem to respect this, at least out of the box. Investigate ways to skip the CI builds when commits are tagged with [skip ci] or at least from certain bot accounts

Additional info: Github Actions supports skipping workflow runs from push and pull_request events when various [skip ci] [ci skip] etc phrases are in the commits (as of Feb 2021)

However, pull_request_target, which we are using, is not skipped. https://docs.github.com/en/actions/guides/about-continuous-integration#skipping-workflow-runs

Note: Skip instructions only apply to the push and pull_request events. For example, adding [skip ci] to a commit message won't stop a workflow that's triggered on: pull_request_target from running.

We are using pull_request_target so that the Slack bot token secret can be passed to the notify_start, notify_success and notify_failure steps. We have the CODECOV_TOKEN in there as well, but I don't think it is actually used since the repository is public.

brandongregoryscott commented 3 years ago

Related: it looks like builds from PRs are actually running from the target branch (main) instead of the code from the source branch, which is definitely not expected behavior. See comment in this recent PR: https://github.com/AndcultureCode/AndcultureCode.Cli/pull/212#discussion_r662954037

Basically, the build on the PR passed despite having changes that would actually break the build once merged.

I am thinking one solution to this is:

  1. Update pull_request_target event to pull_request
  2. Update the notify_start, notify_success and notify_failure jobs to conditionally check to see if the env.SLACK_BOT_TOKEN variable is available before running its steps

We won't get notifications for PR builds anymore, but at least we'll know they are running the correct branch's code. We should still get the notifications when merged into main - which triggers the push event and has access to the repository secrets.

myty commented 3 years ago

Yeah, this is what I'm finding with trying to figure out why the last merge passed tests but then ultimately failed when merged into main. I'm also looking at not running an action on draft PRs.

Related: it looks like builds from PRs are actually running from the target branch (main) instead of the code from the source branch, which is definitely not expected behavior. See comment in this recent PR: https://github.com/AndcultureCode/AndcultureCode.Cli/pull/212#discussion_r662954037

Basically, the build on the PR passed despite having changes that would actually break the build once merged.

I am thinking one solution to this is:

  1. Update pull_request_target event to pull_request
  2. Update the notify_start, notify_success and notify_failure jobs to conditionally check to see if the env.SLACK_BOT_TOKEN variable is available before running its steps

We won't get notifications for PR builds anymore, but at least we'll know they are running the correct branch's code. We should still get the notifications when merged into main - which triggers the push event and has access to the repository secrets.