-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 clean up to fix lint task and remove Windows / MacOS from matrix #3122
Conversation
Changing to draft until I figure out why this is failing again. I thought this was passing after the update but apparently it's hanging on something else. |
…g linux Removes the windows and macos matrix from the CI workflow as they were never actually setting the OS. Both were running against the "ubuntu-latest" OS. Trying to actually use them would not work either as neither windows or macos is supported for service containers. A different means will be needed to test on those platforms. Until that's done, this removes those from the matrix as we were simply running the same thing 3x for the same node versions.
47156c2
to
2ad1efb
Compare
Looks like it was caused by me adding v20 into the matrix. Only that one failed on the pg-native tests. I removed it from the matrix (so it's the same versions as before the PR) and now it's passing. |
Related: #3110 |
Ah! I didn't notice that existing one for fixing the eslint stuff. We should limit dependabot to less open PRs (like maybe just one). It's a lot noise to signal and ends up taking up the entire first screen worth of the PR list... |
well dang i totally missed this PR. Sorry. I have an update I'm going to post on the documentation soon, but I am going to be switching to full time (well probably part time, but my only form of "employment") work on node-postgres soon. May 1st is when I plan on it. So I'll be a lot more responsive & will come out w/ some tutorials and so on...have a bunch of plans. In the mean time I'll still struggle to keep up, but its about to turn around. 😄 Appreciate the help! |
Thanks! CI on master is green now ✔️ https://github.com/brianc/node-postgres/actions/runs/8162362778 |
Couple of commits here to clean up some CI issues.
That last one I ran into while working on something else. The CI for this project was missing this line in the workflow:
It was hard coded to
ubuntu-latest
so our "Windows" and "MacOS" builds were really just running the same tests as Linux 3x. After fixing the lint issue and therun-on
clause, I tried out a real matrix with those OS's and as expected it failed: https://github.com/sehrope/node-postgres/actions/runs/7427371357/job/20212934334This PR does not add back either of them yet as it's kind of complicated to set up Windows to pretend to a be a *nix environment. And from what I've seen so far, the MacOS runners are really slow to setup docker etc.
Figure we should at least get the "remove these dupes" piece in first and can have separate PRs to add those OS platforms into the matrix. May make sense to only run them on a smaller set of node versions as well.
Not sure how anybody could actually review the yarn lock piece of the version bumps. If it's easier, I can pull that out so you can run the install locally and generate the pinned versions.