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

chore: Use new text-based bun lockfile #3846

Merged
merged 3 commits into from
Jan 31, 2025

Conversation

ryoppippi
Copy link
Contributor

@ryoppippi ryoppippi commented Jan 21, 2025

Hello!

Bun introduced new text-based lockfile
https://bun.sh/docs/install/lockfile#text-based-lockfile

Now we can see the diff of deps from this lockfile on Github! so I think we can delete the yarn.lock file
Also, I updated the bun's version for github actions to use this new lockfile

@ryoppippi ryoppippi force-pushed the feature/use-bun-text-lockfile branch from d696723 to 4c28e6f Compare January 21, 2025 20:39
@ryoppippi ryoppippi marked this pull request as draft January 21, 2025 20:39
@ryoppippi ryoppippi force-pushed the feature/use-bun-text-lockfile branch 2 times, most recently from ab94405 to 9bca5a3 Compare January 21, 2025 20:44
@ryoppippi ryoppippi marked this pull request as ready for review January 21, 2025 20:45
bun v1.1.39 is the minimum version to use text-based bun.lock file
@ryoppippi ryoppippi force-pushed the feature/use-bun-text-lockfile branch from 9bca5a3 to 34ef8bf Compare January 21, 2025 20:46
@ryoppippi
Copy link
Contributor Author

ryoppippi commented Jan 21, 2025

I mistakenly formatted ci.yaml so I force pushed

Copy link

codecov bot commented Jan 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.31%. Comparing base (3fba7a5) to head (34ef8bf).
Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3846   +/-   ##
=======================================
  Coverage   91.31%   91.31%           
=======================================
  Files         161      161           
  Lines       10240    10240           
  Branches     2926     2887   -39     
=======================================
  Hits         9351     9351           
  Misses        888      888           
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

Choose a reason for hiding this comment

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

yarn.lock is necessary for using the np. You should not remove it.

@yusukebe
Copy link
Member

Hi @ryoppippi

Thank you for the PR. It's fine to use the text-based Bun lockfile. But as I commented, yarn.lock is necessary for using the np which is used for publishing the package.

@ryoppippi
Copy link
Contributor Author

Hi! thank you for reviewing
Sure, I'll look into it

@jbergstroem
Copy link

Hi @ryoppippi

Thank you for the PR. It's fine to use the text-based Bun lockfile. But as I commented, yarn.lock is necessary for using the np which is used for publishing the package.

Looks like the latest version of np supports bun: https://github.com/sindresorhus/np/releases/tag/v10.1.0

@ryoppippi
Copy link
Contributor Author

@jbergstroem Thanks for your comment.
As you mentioned, np does support bun, but it doesn't support the new bun.lock file, so I made a PR sindresorhus/np#761 . When it is merged, I'll update np package in Hono and send a review request to yusuke-san again

@jbergstroem
Copy link

jbergstroem commented Jan 26, 2025

@jbergstroem Thanks for your comment. As you mentioned, np does support bun, but it doesn't support the new bun.lock file, so I made a PR sindresorhus/np#761 . When it is merged, I'll update np package in Hono and send a review request to yusuke-san again

looks like it was merged and even shipped 🚢! fast moves.

@yusukebe
Copy link
Member

Woooow awesome np supports Bun lock file!

@yusukebe yusukebe changed the title Use new text-based bun lockfile chore: Use new text-based bun lockfile Jan 27, 2025
Copy link
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

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

LGTM!

@yusukebe
Copy link
Member

Hey @ryoppippi

Looks good. I'll merge this now. Thank you!

@yusukebe yusukebe merged commit 0aa8acc into honojs:main Jan 31, 2025
16 checks passed
@ryoppippi ryoppippi deleted the feature/use-bun-text-lockfile branch January 31, 2025 20:08
@ryoppippi
Copy link
Contributor Author

Yes!!!

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