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

Skip Build and Deploy jobs on forks instead of failing them #153

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mattmaniak
Copy link
Member

@mattmaniak mattmaniak commented Jan 8, 2025

Description

Type(s)

  • Game page addition
  • Game page modification
  • Other wiki edit

Verification

Have you

  • Followed the contribution guidelines?
  • Ran ./scripts/generate.mjs?
  • Ran ./scripts/lint.mjs?
  • Checked spelling and grammar?
  • (If applicable) Provided documentation for a game not working? (i.e. detailing what happens when the game doesn't work?)
  • (If applicable) Ensured the linked Steam ID is the correct one for your game addition?

This PR addresses an issue where a push to a forked repo causes error and a notification with such on GitHub. As we build and deploy only from the main repo it may reduce frustration produced by the constant errors sent to the regular contributors. This fix switches from error notifications to "job skipped" notifications.

@hahayupgit
Copy link
Member

I like this addition, with more 'meta' changes like this I'll wait till Isaac provides feedback though. Otherwise I approve!

@@ -16,6 +16,7 @@ concurrency:

jobs:
build:
if: github.repository == 'Whisky-App/whisky-book'
Copy link
Member

Choose a reason for hiding this comment

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

This should just be skipping the deployment step and not the build step. It's still valuable to make sure that build is working as intended on forks.

Copy link
Member Author

@mattmaniak mattmaniak Jan 9, 2025

Choose a reason for hiding this comment

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

I have tried it but it fails on my fork always at least.

Copy link
Member

Choose a reason for hiding this comment

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

Skip the setup pages step, but not the entire job

Copy link
Contributor

@JoshuaBrest JoshuaBrest left a comment

Choose a reason for hiding this comment

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

No? You can just disable them in your repository’s actions…

@mattmaniak
Copy link
Member Author

This is the output of a conditional applied to the Build job:

image

2025-01-10T15:57:40.2446848Z ##[error]Get Pages site failed. Please verify that the repository has Pages enabled and configured to build using GitHub Actions, or consider exploring the `enablement` parameter for this action. Error: Not Found - https://docs.github.com/rest/pages/pages#get-a-apiname-pages-site
2025-01-10T15:57:40.2455054Z ##[error]HttpError: Not Found - https://docs.github.com/rest/pages/pages#get-a-apiname-pages-site

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