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

fix(biome): use temp file instead of stdin #4775

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

redbmk
Copy link
Contributor

@redbmk redbmk commented May 13, 2024

biome handles utf8 characters differently between files and stdin, and in some cases can replace emojis with ascii characters when using stdin

refs: biomejs/biome#2604


This is pretty small but I made sure to update the relevant test. Again, it will have minor conflicts with #4774, #4763, and #4773, but I wanted to keep the fixes focused.

Vader tests and linters are passing locally.

I have another change pending to add biome support for json and jsonc and clean up the options, but it sort of depends on some of these other PRs so I'll wait until these are resolved.

@hsanson
Copy link
Contributor

hsanson commented Jun 17, 2024

Having a hard time figuring out why the Github actions seem to be stuck without any visible way to retry them.... maybe @w0rp can help here.

@redbmk
Copy link
Contributor Author

redbmk commented Jun 20, 2024

@hsanson I think an approval might kick off the tests

At least 1 approving review is required by reviewers with write access.

Could be that it's stale or something though? I'll try pulling in the latest and updating my branch


Edit: I get a slightly different message on mobile that maybe makes it more clear that's what's going on:

1 check awaiting approval

This workflow requires approval from a maintainer.

The other message I saw was unclear if approval was needed for the workflow or to actually merge.

Still not clear if the PR itself needs approval or if there's some other way to approve just the workflow.

biome handles utf8 characters differently between files and stdin, and
in some cases can replace emojis with ascii characters when using stdin

refs: biomejs/biome#2604
Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@hsanson hsanson merged commit 0cd64c8 into dense-analysis:master Jun 20, 2024
7 checks passed
@redbmk redbmk deleted the biome-tempfile branch June 20, 2024 23:20
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.

None yet

2 participants