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

test(common/web/types): unit tests for string-list #12702

Merged

Conversation

markcsinclair
Copy link
Contributor

Unit tests for `ldml-keyboard/string-list.ts'

See #9052

@keymanapp-test-bot skip

@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Nov 26, 2024

User Test Results

Test specification and instructions

User tests are not required

@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S16 milestone Nov 26, 2024
@markcsinclair markcsinclair self-assigned this Nov 26, 2024
@github-actions github-actions bot added test Any acceptance test issue web/ labels Nov 26, 2024
@markcsinclair markcsinclair marked this pull request as draft November 26, 2024 08:13
@github-actions github-actions bot added web/ and removed web/ labels Nov 26, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@markcsinclair
Copy link
Contributor Author

markcsinclair commented Nov 27, 2024

Hmm ... I have been happily typing (developer) on most of the commits ... oops! (... and upsetting the bot too)

@github-actions github-actions bot added web/ and removed web/ labels Nov 27, 2024
@markcsinclair markcsinclair marked this pull request as ready for review November 27, 2024 06:26
@markcsinclair
Copy link
Contributor Author

Ready for review ... as this has largely been done in a single day following a context switch from C++, please check carefully.

@mcdurdin mcdurdin requested a review from srl295 November 27, 2024 08:43
Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -0,0 +1,207 @@
import 'mocha';
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 204 to 205
for (const s of source)
listItem.push(new ListIndex(new StrsItem(s)));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (const s of source)
listItem.push(new ListIndex(new StrsItem(s)));
for (const s of source) {
listItem.push(new ListIndex(new StrsItem(s)));
}

Per style guide once more ("Always use braces and always on a new line")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@github-actions github-actions bot added web/ and removed web/ labels Nov 28, 2024
@github-actions github-actions bot added web/ and removed web/ labels Nov 28, 2024
@markcsinclair markcsinclair merged commit 8e02074 into master Nov 28, 2024
3 checks passed
@markcsinclair markcsinclair deleted the test/common/web/types/9052-unit-tests-string-list branch November 28, 2024 01:49
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 18.0.147-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common/web/ common/ test Any acceptance test issue web/
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants