Skip to content

Conversation

aboqasem
Copy link
Contributor

@aboqasem aboqasem commented Jan 24, 2024

No description provided.

@aboqasem aboqasem force-pushed the refactor/5050-use-bun branch from 7d68638 to 2043bc4 Compare January 24, 2024 08:45
@aboqasem
Copy link
Contributor Author

aboqasem commented Jan 24, 2024

Oh forgot to check the test file haha! Looking into it.

EDIT: I believe it's only because I did not run bun i after setup.

@aboqasem aboqasem force-pushed the refactor/5050-use-bun branch from 4190170 to 68b8eb8 Compare January 24, 2024 23:24
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bun 1.0 doesn't work on windows, which is a show-stopper. Please update to 1.1

@aboqasem aboqasem force-pushed the refactor/5050-use-bun branch from 68b8eb8 to d504211 Compare April 17, 2024 11:10
@aboqasem aboqasem requested review from ematipico and nhedger April 17, 2024 11:12
@aboqasem
Copy link
Contributor Author

tsc —noEmit is failing on both node and bun, will check later

@aboqasem aboqasem force-pushed the refactor/5050-use-bun branch from 0241312 to 7dc1d0f Compare April 17, 2024 14:19
@aboqasem
Copy link
Contributor Author

That should be it :)

@nhedger
Copy link
Member

nhedger commented Apr 17, 2024

This pull request is welcome because using Bun as a bundler and a package manager here allows us to remove a bunch of dependencies. Still, I'm not sure this solves #44 because the original intent was to use Bun to run the script.

aboqasem and others added 3 commits April 18, 2024 06:50
Co-authored-by: Nicolas Hedger <[email protected]>
Co-authored-by: Nicolas Hedger <[email protected]>
Co-authored-by: Nicolas Hedger <[email protected]>
@aboqasem
Copy link
Contributor Author

aboqasem commented Apr 17, 2024

Still, I'm not sure this solves #44 because the original intent was to use Bun to run the script.

Yeah agree, will update the description.

We can add a Dockerfile to run the action with Bun, but I believe it would be slower because of fetching the Docker image instead of using the existing Node runner provided by GitHub. No?

@aboqasem aboqasem changed the title refactor: use bun refactor: use bun for development Apr 17, 2024
@aboqasem aboqasem requested a review from nhedger April 17, 2024 23:16
@nhedger nhedger merged commit 8428e61 into biomejs:main Apr 18, 2024
@nhedger
Copy link
Member

nhedger commented Apr 18, 2024

Not a fan of the Docker way either.

Thanks for your contribution !

@aboqasem aboqasem deleted the refactor/5050-use-bun branch April 18, 2024 07:59
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.

3 participants