ci: replace TravisCI with GitHub Actions#483
Conversation
d25edb1 to
375ea7d
Compare
type: fix failures in TS 3.x+ where "toString doesn't exist on T" typescript: show failures starting in 3.1 ci: only run against "working" verions of TypeScript
9e64247 to
6f14773
Compare
|
Let me know what you think about the questions I left up @markerikson and @phryneas |
|
See passing CI runs here: https://github.com/eXamadeus/reselect/actions (and all the failing ones from my testing...haha) |
|
Yipes. That's... lovely :) Obviously my goal here was that we'd do the CI switch, see the current code building okay, and then work on any further updates. I don't have an immediate suggestion here, tbh. I know Lenz has been kinda burned out lately, so while I'd really like to get his feedback on this, not sure if he's feeling up to it atm. I'll try putting out the bat-signal on Twitter and see if anyone else has suggestions. |
Looks like this was already done. I agree with it as only Node 10+ is currently supported.
Yup, it should be automatic. |
nickserv
left a comment
There was a problem hiding this comment.
Looks good overall, though I'm not sure if we need both Coveralls and Codecov, they both gather code coverage data.
Yeah I just brought over everything that was in Travis. I didn't want to remove anything without confirmation. Which would be preferred you think? (Edit: I think codecov.io affects PRs, so that might be the one to keep right? I'm not sure it triggered though, I might need more config to get that running)
Also, I'm a dope. I totally changed the node versions already. What I meant to say is "are these the ones we should support" or something haha! |
|
I'm not sure. It may be safer to leave them both in, unless another maintainer can confirm if only one is needed. |
|
OK so I think I verified that Codecov.io is uploading correctly: https://codecov.io/github/eXamadeus/reselect/commit/9626793b028b2872bca2a02cc3353ba6c7ad10ad Although I'm not entirely convinced, because I would have expected a comment on this PR (or a "check")...any ideas how to set that up? The docs for the action don't lead me to believe there is a configuration value I'm missing. |
…) also allow non-master reports
|
Hmm, well Codecov comments are working on my "testing" PR: eXamadeus#1 Not sure if this is an issue with forked repos vs "natural branches" on the targeted repo... EDIT: This is likely just because there is no branch in this repo ( |
|
@nickmccurdy Are you familiar with the comment setup for Codecov? I made some small changes to make sure Codecov reports when PRs are made. Let me know if those settings make sense. I decided to turn off the "master only" filter so branches like |
|
Yeah, if the build+test steps are working here, I'm fine with getting this in as a baseline. We can always double-check code coverage behavior later and fix it if necessary. |
|
Skimming the settings, seems reasonable to my not-familiar-with-GH-actions eye. You happy with this as-is? |
|
Okay, let's get this in and keep going, then. |
Replace TravisCI for GitHub Actions
This pulls inspiration from https://github.com/reduxjs/redux-toolkit/blob/next/.github/workflows/tests.yml and spawned from conversations in #480.
Fixes #482
Notes to check up on:
are the number of TS versions correctthis is interesting, see the Notes sectionNotes
I was going to update TypeScript to 3.5 (and only test past there) as we discussed in #480 but it looks like there is an interesting bug with TypeScript (or maybe a fix that seems like a bug) where the array typings used by
reselectto type the combiner...well just don't work as expected past TS 3.1...but that makes sense. We should really do that in the type update PR anyway (the 5.0 release).More info...
I have the failures showing here in CI: https://github.com/eXamadeus/reselect/actions/runs/546546429