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

Prevent linting jobs from running on PR close events #1328

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

klutchell
Copy link
Contributor

@klutchell klutchell commented Jan 16, 2025

Prevent linting jobs from running on PR close events

There is no value in running actionlint or pre-commit checks
on PR merge or close events.

Same with the Node version checks, no need on close events.

The octoscan checks should run on merge to update the main
branch code scanning results, but not on PR close.

Tested close event here.
Note that File List runs without failing, so we can leverage it for Custom always job conditions in a future PR.

Use shallow checkout with github token for linting jobs

These jobs do not rely on versioned source, so they
can default to the pull request HEAD sha and fallback
to the github ref for other event types.

Using the github ref is not appropriate for pull_request_target
events where the ref/sha is always the base branch.

Copy link
Contributor

flowzone-app bot commented Jan 16, 2025

Website deployed to CF Pages, 👀 preview link https://59da137c.flowzone.pages.dev

@klutchell klutchell closed this Jan 16, 2025
@klutchell klutchell reopened this Jan 16, 2025
There is no value in running actionlint or pre-commit checks
on PR merge or close events.

Same with the Node version checks, no need on close events.

The octoscan checks should run on merge to update the main
branch code scanning results, but not on PR close.

Change-type: patch
Signed-off-by: Kyle Harding <[email protected]>
flowzone.yml Outdated
@@ -1890,12 +1908,18 @@ jobs:

# check if the repository has a package.json file and which engine versions are supported
is_npm:
name: Check NodeJS versions
name: request branch for pull requestJS versions
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: request branch for pull requestJS versions
name: request branch for pull request JS versions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That must have been a copy-paste error, I didn't even mean to update that job name? Weird.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought it was weird too, but I assumed you had some reason? :p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's very nice of you

These jobs do not rely on versioned source, so they
can default to the pull request HEAD sha and fallback
to the github ref for other event types.

Using the github ref is not appropriate for pull_request_target
events where the ref/sha is always the base branch.

Change-type: minor
Signed-off-by: Kyle Harding <[email protected]>
Copy link
Contributor

@pipex pipex left a comment

Choose a reason for hiding this comment

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

LGTM

@flowzone-app flowzone-app bot merged commit 1830dfc into master Jan 17, 2025
68 checks passed
@flowzone-app flowzone-app bot deleted the kyle/close-events branch January 17, 2025 14:11
@klutchell
Copy link
Contributor Author

As intended, octoscan and check nodeJS versions still runs on merge, backed by File List.

However linting jobs like pre-commit and actionlint are skipped as planned.

image

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