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

Make headword and other utils writing-system aware #1393

Merged

Conversation

myieye
Copy link
Contributor

@myieye myieye commented Jan 20, 2025

This PR moves utils like headword and pickBestAlternative into a new WritingSystemService class, so that the utils are "writing-system aware" and can make more accurate choices based on the project's current writing-system configuration.

@myieye myieye marked this pull request as ready for review January 20, 2025 09:43
Copy link

github-actions bot commented Jan 20, 2025

UI unit Tests

12 tests   12 ✅  0s ⏱️
 4 suites   0 💤
 1 files     0 ❌

Results for commit 85f6063.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 20, 2025

C# Unit Tests

104 tests   104 ✅  5s ⏱️
 16 suites    0 💤
  1 files      0 ❌

Results for commit 85f6063.

♻️ This comment has been updated with latest results.

@hahn-kev hahn-kev self-requested a review January 21, 2025 02:00
Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

looks great! I left some suggestions, they're mostly optional though

@myieye myieye force-pushed the make-entry-and-multi-string-utils-writing-system-aware branch from 02f4dee to 85f6063 Compare January 22, 2025 16:57
@myieye myieye merged commit bee6fb2 into develop Jan 22, 2025
21 checks passed
@myieye myieye deleted the make-entry-and-multi-string-utils-writing-system-aware branch January 22, 2025 19:36
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.

2 participants