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: skip leading bom if exist #166

Merged
merged 3 commits into from
Jan 27, 2025
Merged

fix: skip leading bom if exist #166

merged 3 commits into from
Jan 27, 2025

Conversation

kinkelpk
Copy link
Contributor

@kinkelpk kinkelpk commented Jan 23, 2025

This closes #165

@tisonkun
Copy link
Member

I've triggered the CI workflow for you.

I'm not quite familiar with CSharp's BOM mechanism. A rough thought is to detect those characters and skip them always.

@kinkelpk
Copy link
Contributor Author

Thx for triggering .Added exclusion for newly added folder, to get past some of the checks.
I'm afraid i'm not too familiar with github workflows. Is there any way fo me to retrigger ?

@tisonkun
Copy link
Member

tisonkun commented Jan 23, 2025

@kinkelpk This repo has a setting to require approval for first-time contributor's PR.

You can ping me to retrigger, or open a PR against your own fork for self-serve debugging. It's not needed for your following PRs.

@kinkelpk
Copy link
Contributor Author

Ok looking better now. At least 2/3 smoke tests fail for the right reason now :)

I'm afraid i wont be of much help for a fix. Rust is too unfamiliar for me. I was struggeling to set up some sort of unit/int test. I hope this smoke testcase is still some sort of use.

Signed-off-by: tison <[email protected]>
@tisonkun tisonkun changed the title test: added test for bom issue fix: skip leading bom if exist Jan 27, 2025
@tisonkun
Copy link
Member

I made an in-place fix. Let's see.

You can test it locally with cargo install --path cli and hawkeye ...

@kinkelpk
Copy link
Contributor Author

sweet! problem solved! Thanks!

@tisonkun tisonkun merged commit 223367e into korandoru:main Jan 27, 2025
17 checks passed
@tisonkun
Copy link
Member

Thanks for your contribution!

@Lorilatschki
Copy link

@tisonkun thanks for your support! Any idea when this is deployed to the next release?

@tisonkun
Copy link
Member

It's Chinese New Year this week and the next. I may schedule a release after this festival period. Feel free to ping me after that if I don't cut one.

@Lorilatschki
Copy link

Thanks for the fast response. Happy Chinese New Year celebration.

@tisonkun
Copy link
Member

@Lorilatschki today I woke up early and finish the release, lol.

It's now available as 6.0.0. You can check the CHANGELOG for other changes alongside.

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.

Incorrect Placement of BOM When Adding Copyright Headers
3 participants