Skip to content

Conversation

@kianmeng
Copy link
Contributor

@kianmeng kianmeng commented Dec 30, 2025

Short title (summary): Fix typos

Description
Found via codespell -S ./build,./benchmarks/competition -L vie,persan,fo,ans,larg,indx,shft,carryin and typos --hidden --format brief

Type of change

  • Bug fix
  • Optimization
  • New feature
  • Refactor / cleanup
  • Documentation / tests
  • Other (please describe):

How to verify / test

  • Add additional tests to verify bugs or new features.
  • If you claim performance gains, you should provide benchmark numbers using high quality benchmarking code.

Please read before contributing:

If you can, we recommend running our tests with the sanitizers turned on.
For non-Visual Studio users, it is as easy as doing:

cmake -B build -D SIMDUTF_SANITIZE=ON
cmake --build build
ctest --test-dir build

Our CI checks, among other things, for trailing whitespace.

Checklist before submitting

  • I added/updated tests covering my change (if applicable)
  • Code builds locally and passes my check
  • Documentation / README updated if needed
  • Commits are atomic and messages are clear
  • I linked the related issue (if applicable)

Final notes

  • For large PRs, prefer smaller incremental PRs or request staged review.

Thanks for the contribution!

}
#endif
#define SIMDUTF_GET_CURRENT_IMPLEMENTION
#define SIMDUTF_GET_CURRENT_IMPLEMENTATION
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not being used anywhere?

$ rg SIMDUTF_GET_CURRENT_IMPLEMENTION
src/implementation.cpp
1425:#define SIMDUTF_GET_CURRENT_IMPLEMENTION

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems like it can be removed entirely!

@kianmeng kianmeng mentioned this pull request Dec 30, 2025
@pauldreik
Copy link
Collaborator

thanks!
I think this is good, but maybe the external code in benchmark should be left untouched? let's see what @lemire says. the justification is that it is a reference, and it is good if the reference is as close as possible to the original in case one needs to investigate differences to later versions (or what do I know, I figure someone deep diving into performance regressions might be helped).

@lemire
Copy link
Member

lemire commented Dec 30, 2025

@kianmeng Any chance you might omit benchmarks/competition/*. It is vendored code that we would prefer to modify as little as possible.

If it is inconvenient, we may still merge this PR...

Found via `codespell -S ./build,./benchmarks/competition -L vie,persan,fo,ans,larg,indx,shft,carryin`
and `typos --hidden --format brief`
@kianmeng kianmeng reopened this Jan 2, 2026
@kianmeng
Copy link
Contributor Author

kianmeng commented Jan 2, 2026

@lemire @pauldreik benchmarks/competition/* excluded and PR updated..

@kianmeng kianmeng requested a review from pauldreik January 2, 2026 17:12
@pauldreik
Copy link
Collaborator

pauldreik commented Jan 2, 2026

looks good!
to fix the clang format issue, go to the job, go to the "store patch as artifact if there is one", download it (the link is https://github.com/simdutf/simdutf/actions/runs/20662762578/artifacts/5009537180 ) and then paste it on stdin to "git apply".

@lemire
Copy link
Member

lemire commented Jan 2, 2026

@pauldreik Although we can merge and then lint. It is not a very challenging problem. :-)

@kianmeng
Copy link
Contributor Author

kianmeng commented Jan 5, 2026

@pauldreik Clang patch applied, ddd1a28

@pauldreik pauldreik merged commit d56ea3e into simdutf:master Jan 5, 2026
81 checks passed
@pauldreik
Copy link
Collaborator

Thanks @kianmeng !

@kianmeng
Copy link
Contributor Author

kianmeng commented Jan 6, 2026

🥳 🥳 🥳 🥳 🥳

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