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: Update workflows #1506

Draft
wants to merge 40 commits into
base: master
Choose a base branch
from
Draft

CI: Update workflows #1506

wants to merge 40 commits into from

Conversation

tooomm
Copy link
Contributor

@tooomm tooomm commented Mar 26, 2021

  • Restructure CI build matrix and use in-file defined Node versions

    • Remove hardcoded Node versions from CI
    • Use defined Node version in the project (decide on only one?)
    • Add LTS-1 release alias (maintenance LTS)
    • Add LTS release alias (active LTS)
    • Add Current release alias to verify future LTS compatibility
  • Miscellaneous

    • No longer run postinstall scripts twice in each job, saves 30% time
    • Disable fast failing (already done)
    • Use npm ci over npm i (already done)
    • Update actions to setup-node v3 and checkout v3 (already done)
    • Update actions to setup-node v4 and checkout v4 (already done)
    • Utilize npm cache capability from setup-node action
    • Use Node version from .nvmrc file for ESLint job (or could be package.json/enginges, same as above)
    • Workaround for Node 14 seems not to be needed anymore (removed by now)
    • Improve job+step naming and have more isolated script runs
    • Split module installation and scripts in Dockerfile
    • Print various information about Docker image/container
    • Comment out Docker from dependabot config (See Dockerfile: Use Node LTS-alpine #1505)
    • README: The build script is already run as part of the postinstall script

Further improvements:

  • Node version reported from docker container seems off?
  • "docker sbom IMAGE" could contain helpful information, still experimental

@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-gvmkvy March 26, 2021 22:19 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-vqydas March 26, 2021 22:25 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-8xlyzw March 26, 2021 22:27 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-vqzt7n April 12, 2021 12:33 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-dysgso April 12, 2021 12:38 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-8tu7lw April 12, 2021 12:46 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-2j3bc2 April 12, 2021 12:48 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-fwmovo April 12, 2021 18:04 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-k92pts April 12, 2021 18:08 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-c0j7ya April 12, 2021 18:12 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-aiddbz April 12, 2021 18:14 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-bjjrsl April 12, 2021 18:18 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-wu3pgc April 12, 2021 18:21 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-c4tbdl April 12, 2021 18:23 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-4tpkso April 12, 2021 18:27 Inactive
@HerveH44 HerveH44 temporarily deployed to beta-dr4ft-tooomm-patch-f2pt1r April 12, 2021 18:30 Inactive
@mixmix
Copy link
Member

mixmix commented Feb 26, 2022

Hey @tooomm apologies that I fell off responding to GitHub mentions and helping review - had a baby 2 years ago and only now starting to unlock spare time again!

Lmk if you'd like to jam on any of this

@ZeldaZach
Copy link
Member

I can probably figure this out with you @mixmix when you're ready. Hmu on discord

@tooomm
Copy link
Contributor Author

tooomm commented Feb 28, 2022

Hey @tooomm apologies that I fell off responding to GitHub mentions and helping review - had a baby 2 years ago and only now starting to unlock spare time again!

That are both great news. I guess I can only congratulate you then! 😉🥳

Lmk if you'd like to jam on any of this

I updated the PR with the newest changes from master.
However, I'm not quite sure why the job for Node 12 fails. Same issue on master.
But Node 12 is EOL anyway and we might want to upgrade: #1616


Node current is expected to fail btw. I'm waiting for an open PR to be merged in the setup-node repo that adds this feature.
It's available now.

@tooomm tooomm marked this pull request as ready for review May 27, 2022 08:22
@tooomm tooomm marked this pull request as draft June 4, 2022 16:50
@mixmix
Copy link
Member

mixmix commented May 30, 2023

@tooomm saw you pushed an update on this. It sucks that CI is being such a pain (both in master branch and here).
Looking at the tests failing here I don't think it's your fault for Node.js (.nvmrc) Docker (pull_request). Not sure about the middle one.

@tooomm
Copy link
Contributor Author

tooomm commented Oct 29, 2023

Yes, it's a bit annoying. Guess that's why you looked into #1702 as well. :)

I did update this PR, too. Not sure what's the best practice to define a projects Node version or if we want to keep both:

  • Node 16 (.nvmrc definition) runs the postinstall script and fails in follow up test step. If you rerun several times, it might as well pass it... 🙄
  • Node 18 (package-lock range) fails - while running the same steps - during postinstall script already
  • Docker with image node:lts-alpine uses Node 20 currently and fails at the same script

I assume this is an incompatibility issue with more recent Node versions. (And the tests are flaky)

It's the same situation on master right now:
https://github.com/dr4fters/dr4ft/actions/runs/6676321403/job/18145256056

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.

4 participants