search

Wakamai-Fondue / wakamai-fondue-site

Wakamai Fondue website
Apache License 2.0
45 stars 8 forks source link

chore: update CI action versions #186

Closed mxdvl closed 1 month ago

mxdvl commented 1 month ago

And link Node’s version to the one in repo

RoelN commented 1 month ago

@mxdvl Thanks for your PR! Currently the CI is failing on all kinds of Prettier warnings. Are these a side effect of your changes?

mxdvl commented 1 month ago

It must be, somehow, but I'm not sure why… ☹️

Does the vue lint command work locally on your machine with Node 16.17?

Ryuno-Ki commented 1 month ago

node_modules/.bin/prettier --write . edits a bunch of files here when I check out your branch, @mxdvl

RoelN commented 1 month ago

I'm okay with the changes Prettier suggests. As long as the codebase is consistent. Can we let Prettier just fix them?

RoelN commented 1 month ago

Also I wonder if this PR will run into the same ssh/https trouble as seen here: https://github.com/Wakamai-Fondue/wakamai-fondue-site/actions/runs/11230829820/job/31218950314?pr=185

I'm trying to fix that here: https://github.com/Wakamai-Fondue/wakamai-fondue-site/pull/187 The problem is apparently that we used to load the Engine dependency over ssh while developing it before making everything public. Since the repo is public now we can just load it over https instead of ssh. My commit there seems to fix that particular error, now stranding on another. Oh well, diving into that one now!

mxdvl commented 1 month ago

node_modules/.bin/prettier --write . edits a bunch of files here when I check out your branch, @mxdvl

I'm okay with the changes Prettier suggests. As long as the codebase is consistent. Can we let Prettier just fix them?

Thanks both, have done so in 4a95abdccefd2e268a6b0eba7baca55f84f8a63b, but the linter wanted something slightly different, so you can find the result of npm run lint in 0a91d50f9a704a9d419b8bf49dc5e04a54a9a048

RoelN commented 1 month ago

@mxdvl I get the error

npm ERR! Missing script: "lint:check"

Is that something that you added but didn't commit?

RoelN commented 1 month ago

Oh I just noticed https://github.com/Wakamai-Fondue/wakamai-fondue-engine/pull/66 which might be relevant?

mxdvl commented 1 month ago

Oh no I thought I'd reverted that — fix incoming. It was only a way of ensuring we could run the exact same check locally as in CI, but it'll extract it to its own PR.

RoelN commented 1 month ago

Making progress! \o/

I see it now complains about filenames, which could be fixed by cherry-picking https://github.com/Wakamai-Fondue/wakamai-fondue-site/pull/187/commits/5a9bdec33e987c5ad94f5c8ca86c4e8a7f153fb8 where @Ryuno-Ki addressed this. Let's do this, and then when this PR is green, rebase it against the other PRs?

mxdvl commented 1 month ago

Oh I just noticed Wakamai-Fondue/wakamai-fondue-engine#66 which might be relevant?

This is another approach that we could go with, using a matrix of Node versions, but I think that having a consistent version locally and on CI is a simpler first step.

I see it now complains about filenames, which could be fixed by cherry-picking 5a9bdec where @Ryuno-Ki addressed this. Let's do this, and then when this PR is green, rebase it against the other PRs?

Done ✅

RoelN commented 1 month ago

Ha! Nice, we're at the old zlib/fs error, which will be addressed in @Ryuno-Ki's PR!

If nobody objects, perhaps we can get this merged into the master, then rebase the master in your branch, @Ryuno-Ki?

Ryuno-Ki commented 1 month ago

Coolio. Now @RoelN has to decide to either adopt #184 or #185.

I vote for merging this PR and continue there.

(what a :yarn: of issues :fearful: )

Ryuno-Ki commented 1 month ago

Thanks, @mxdvl !

RoelN commented 1 month ago

And... done!!

Thanks @mxdvl and @Ryuno-Ki for your help, it is much appreciated!