Skip to content

Conversation

myieye
Copy link
Collaborator

@myieye myieye commented Sep 8, 2025

Before:
image

After: (It would be easy to also show the current audio field if that seemed advantageous.)
image

Copy link

coderabbitai bot commented Sep 8, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch poc-optimize-audio-context

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the 💻 FW Lite issues related to the fw lite application, not miniLcm or crdt related label Sep 8, 2025
Copy link

github-actions bot commented Sep 8, 2025

UI unit Tests

  1 files  ±0   41 suites  ±0   19s ⏱️ +2s
 90 tests ±0   90 ✅ ±0  0 💤 ±0  0 ❌ ±0 
124 runs  ±0  124 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 8610d28. ± Comparison against base commit d8051b6.

♻️ This comment has been updated with latest results.

[InlineData("en-Zxxx-x-audio", "en")]
[InlineData("seh-Zxxx-x-audio-var", "seh-x-var")]
[InlineData("lwl-Zxxx-x-majority-audio", "lwl-x-majority")]
[InlineData("lwl-Latn-x-majority", "lwl-x-majority")]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The script is only relevant to text WS's. So, if there are multiple WSs where only the script differs we show all of them.

Copy link

argos-ci bot commented Sep 8, 2025

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ✅ No changes detected - Sep 8, 2025, 1:52 PM

Copy link

github-actions bot commented Sep 8, 2025

C# Unit Tests

130 tests  ±0   130 ✅ ±0   19s ⏱️ ±0s
 20 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 8610d28. ± Comparison against base commit d8051b6.

♻️ This comment has been updated with latest results.

@myieye myieye force-pushed the poc-optimize-audio-context branch from 73db81f to 8610d28 Compare September 8, 2025 13:49
IsAudio = script?.Equals(WellKnownSubtags.AudioScript, StringComparison.OrdinalIgnoreCase) == true &&
variants?.Split('-').Any(v => v == WellKnownSubtags.AudioPrivateUse) == true;

if ((script is not null || IsAudio) && IetfLanguageTag.TryGetSubtags(code,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TryGetSubtags appears to be expensive in some cases perhaps that's, because in some cases it requires the SLDR to be initialized. I'm not sure when that's the case, but it's not the case for any of the test cases I wrote. So, perhaps it's never the case for codes that have a script 🙃

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.

one concerns I have with this approach is that it assumes there will be a matching non audio writing system, and that the field will have data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💻 FW Lite issues related to the fw lite application, not miniLcm or crdt related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants