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 improvements #110

Merged
merged 2 commits into from
Feb 7, 2021
Merged

CI improvements #110

merged 2 commits into from
Feb 7, 2021

Conversation

tancredi
Copy link
Owner

@tancredi tancredi commented Feb 7, 2021

No description provided.

XhmikosR and others added 2 commits January 22, 2021 23:11
* change on event to `pull_request`
* specify `FORCE_COLOR: 2`
* update to `actions/setup-node@v2`
* use `npm ci`
* add OS-dependent caching
* skip the release workflow if the repo isn't `tancredi/fantasticon`
* add `fail-fast: false`
@tancredi tancredi merged commit 4a80a54 into master Feb 7, 2021
@tancredi tancredi deleted the patch-2 branch February 7, 2021 13:45
@github-actions
Copy link

github-actions bot commented Feb 7, 2021

🎉 This PR is included in version 1.0.32 🎉

The release is available on:

Your semantic-release bot 📦🚀

@XhmikosR
Copy link
Contributor

XhmikosR commented Feb 7, 2021

@tancredi there's no matrix.os in lint

@XhmikosR
Copy link
Contributor

XhmikosR commented Feb 7, 2021

Also, you could probably remove the cache fallback line ${{ matrix.os }}-node-v${{ env.NODE }}-

@tancredi
Copy link
Owner Author

tancredi commented Feb 7, 2021

Oopsy - #112

TBH I'd almost remove fallbacks entirely - i'd expect cache keys to be either correct or not, but it kinda just makes the thing more unreadable in a way. What do you think?

@XhmikosR
Copy link
Contributor

XhmikosR commented Feb 7, 2021

I'd say just remove the aforementioned fallback and keep the exact match. Better have a cache miss that cache growing, assuming you it's not a lot slower.

@tancredi
Copy link
Owner Author

tancredi commented Feb 7, 2021

But why that one specifically and not others?

@XhmikosR
Copy link
Contributor

XhmikosR commented Feb 7, 2021

The other one is not a fallback. It's needed so that the cache action knows what to restore AFAICT.

@tancredi
Copy link
Owner Author

tancredi commented Feb 7, 2021

Meh - still not totally convinced as I think caching here is far less critical than maintainability and deffo don't see the point in a fallback key (I'd either hit exact key or not hit anything ay all), for the time being just removed that line :)

@XhmikosR
Copy link
Contributor

XhmikosR commented Feb 7, 2021

Like I said, the only fallback there is is that line wherever repeated. The other line is needed for caching. If you don't want caching, that's another thing, but it definitely speeds things up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants