-
-
Notifications
You must be signed in to change notification settings - Fork 640
Skip CI builds when making changes to docs #2518
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
Conversation
This change should make CI ignore changes to files inside the website folder.
|
I don't think this is a useful change. If someone deletes an existing doc for an API that still exists in the Lua code, this change would make it so that change isn't detected and our CI job never attempts to fail that change. |
| - 'website/**' | ||
| pull_request: | ||
| paths-ignore: | ||
| - 'website/**' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can also include *.txt, *.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean txt and md files in the entire repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, modifying text file doesn't impact build
Ah right, I missed that it also runs Do you think it would be useful to instead only run the linux builds since they are fast and have good availability? That way docs-check is still run. Alternatively we could only skip Mac, since Mac appears to be especially prone to timeout. |
Maybe split the CI, one for code check, one for doc-check? |
I don't really see the use for that. We still have to build premake for the doc check. By that point, we've built it, tests are relatively cheap. If we just say "hey, let's use XYZ runner to do doc-check on", we're just doing extra work. If we're trying to get around the fact that "but maybe X is degraded, so we want to use Y", that only works if "Y" is never degraded. |
|
If there's no appetite for this change I'm happy to close it, but I think this point is not quite correct:
Currently all jobs are being run for website changes and the PR is blocked if any of them fail. If we were to only do Linux builds for website changes then it would eliminate multiple points of failure that are currently adding no value (unless of course I'm missing something which is entirely possible). This would not require any logic for switching runners depending on what is degraded. It would also mean CI completes faster overall. Skipping Windows avoids the slowest running job, and skipping Mac avoids the slowest to allocate job and the most likely source of failure due to being unavailable. Sure, Linux runners could also become unavailable for a while. This change would not avoid such a situation, but it wouldn't make it worse either. |
|
Given we don't pay for runners, I have no issue with them taking a little extra time to ensure its working on all platforms as expected. |
|
Closing this PR at this time. We may re-open at a later point. |
What does this PR do?
This change should make CI not trigger when PRs only make changes to files inside the website folder.
Triggering CI for website changes can cause unnecessary delay due to waiting for jobs to complete or jobs being canceled due to no runners being available.
How does this PR change Premake's behavior?
This should not affect the behaviour of Premake itself. Only CI.
Anything else we should know?
I'm not 100% confident in this change since I don't use Github Actions all that often.
It could also be the case that there is some reason to trigger the builds on website changes that I'm not aware of?
The motivation for this change came from submitting this PR, however it looks like that PR failed some CI jobs for reasons that aren't clear to me?
Did you check all the boxes?
closes #XXXXin comment to auto-close issue when PR is merged)You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!