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(developer): keyboard info compiler messages unit tests 2 #11253

Merged

Conversation

markcsinclair
Copy link
Contributor

@markcsinclair markcsinclair commented Apr 18, 2024

Some tidying up of earlier unit tests, mainly introducing some constants.

Continues earlier PR #10848

@keymanapp-test-bot skip

@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Apr 18, 2024

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

@keymanapp-test-bot keymanapp-test-bot bot added this to the B17S6 milestone Apr 18, 2024
@markcsinclair markcsinclair marked this pull request as draft April 18, 2024 10:25
@markcsinclair markcsinclair self-assigned this Apr 18, 2024
@markcsinclair markcsinclair marked this pull request as ready for review April 22, 2024 09:41
const kmpFilename = makePathToFixture('khmer_angkor', 'build', 'khmer_angkor.kmp');
const jsFilename = KHMER_ANGKOR_JS;
const kpsFilename = KHMER_ANGKOR_KPS;
const kmpFilename = KHMER_ANGKOR_KMP;

const sources = {
Copy link
Member

Choose a reason for hiding this comment

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

You could do this as:

const sources = {
  ...KHMER_ANGKOR_SOURCES,
  sourcePath: 'release/k/invalid-license',
};

here, and above, and that way eliminate a bunch of repeated constants. 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found only a couple of places I could do this; can you spot any more?

Copy link
Member

Choose a reason for hiding this comment

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

didn't spot any more!

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, nice and clean

@markcsinclair markcsinclair merged commit 4ae7418 into beta Apr 25, 2024
4 checks passed
@markcsinclair markcsinclair deleted the test/developer/keyboard-info-compiler-messages-unit-tests-2 branch April 25, 2024 13:07
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.314-beta

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

Successfully merging this pull request may close these issues.

3 participants