Skip to content

chore: consolidate CI stages #1433

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore: consolidate CI stages #1433

wants to merge 2 commits into from

Conversation

43081j
Copy link
Collaborator

@43081j 43081j commented Jun 15, 2025

No description provided.

@43081j 43081j requested a review from paulmillr June 15, 2025 21:08
@paulmillr
Copy link
Owner

It would be better to reuse jsbt actions https://github.com/paulmillr/noble-hashes/tree/main/.github/workflows

there is test-js and test-ts, difference is in: one tests compiled js code, other raw ts. (We should prob use test-js)

@43081j
Copy link
Collaborator Author

43081j commented Jun 15, 2025

Makes sense

Maybe let's merge this one for now and I'll take a look at moving it to jsbt in a follow up?

It looks pretty straight forward so can probably have a look tomorrow

@paulmillr
Copy link
Owner

The changes aren’t compatible — the next one would 100% replace the first one. So, doesn’t make sense to merge both.

@43081j
Copy link
Collaborator Author

43081j commented Jun 16, 2025

@paulmillr ok i've switched it

deno fails on windows:
https://github.com/paulmillr/chokidar/actions/runs/15681757550/job/44174662191?pr=1433#step:4:7

we need a cross-platform way of conditionally installing it i guess. or just skip windows for that step

prettier also fails on windows:
https://github.com/paulmillr/chokidar/actions/runs/15681757550/job/44174662223?pr=1433#step:6:16

this one is odd since the formatting seems to differ between windows and linux. line endings maybe?

@paulmillr
Copy link
Owner

@43081j I see you’re doing matrix in matrix. This is unnecessary: let’s just leave jsbt matrix. This way deno would only be tested once. It could also fix prettier

@43081j
Copy link
Collaborator Author

43081j commented Jun 16, 2025

because we need to use windows, linux and mac here but jsbt only uses one OS at a time

@paulmillr
Copy link
Owner

Hmmm that's my screwup; i've thought matrix in jsbt has windows and mac as well.

Well...

@43081j
Copy link
Collaborator Author

43081j commented Jun 16, 2025

no worries, let me know if you want any help updating that

i suppose we will still need to add a condition around the deno stuff

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.

2 participants