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

CI: remove --check-files and add conditional to prevent duplicate run #1042

Merged
merged 5 commits into from Aug 31, 2020

Conversation

thedavidprice
Copy link
Contributor

@thedavidprice thedavidprice commented Aug 31, 2020

Re-added --check-files, which was removed due to actions/cache infrequently giving a false positive "cache hit" on Windows runner. The problem is that the Windows runner has reverted back to taking > 10 min for each node version. There have been a couple of updates to actions/cache, so I'm curious if there's been any improvement on Windows.

Looking into the Windows long run time specifically, there is an issue with Yarn giving a warning "There appears to be trouble with your network connection. Retrying...", which seems to account for half of the runtime. There's a lot of conversation about this in Yarn Package GH Issues. This comment seems to be the most helpful as a potential next step to fix: yarnpkg/yarn#5259 (comment) I think it would require creating a .yarnrc file in root with the registry and HTTP changes suggested.

[Update]
Also added something I've been wanting to try --> a conditional so that the whole Action will not re-run when a PR is merged.

Problem: because this triggers on both Push and specific PR events, when a PR is merged, the action is effectively re-triggered. We need to have the Push trigger because there are some occasional instances when a direct push to main is needed.

Solution: I added the conditional if: github.event.pull_request.merged == false at the beginning of the jobs. This should allow a direct push to trigger the full runner. However, if the trigger is due to a PR merge event, then the action will trigger but the jobs will not run.

Copy link
Contributor

@peterp peterp left a comment

Choose a reason for hiding this comment

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

🚀

@thedavidprice thedavidprice merged commit 3bac838 into main Aug 31, 2020
@thedavidprice thedavidprice deleted the dsp-CI-cache-check-files branch August 31, 2020 21:13
@thedavidprice thedavidprice added this to the v0.17.0 milestone Aug 31, 2020
@thedavidprice thedavidprice changed the title remove --check-files from CI yarn install CI: remove --check-files and add conditional to prevent duplicate run Aug 31, 2020
@thedavidprice
Copy link
Contributor Author

Not sure if the conditional is working as is. Need to watch for Actions running full jobs process when PR is merged.

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.

There appears to be trouble with your network connection. Retrying...
2 participants