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

refactor(developer): replace VERIFY_KEYBOARD_VERSION() calls with VerifyKeyboardVersion() 🌋 #12063

Merged

Conversation

mcdurdin
Copy link
Member

Remove the VERIFY_KEYBOARD_VERSION() and VERIFY_KEYBOARD_VERSION_ret() macros, and instead use VerifyKeyboardVersion(). The DRYing out of this function call pattern moved the return statement into a macro, which is an inappropriate way to reduce repetition, because (a) it is hard for devs to spot function exits, and (b) automated code analysis tools stumble on it.

Also means that we uncover more clearly the return mismatches between KMX_DWORD and KMX_BOOL through the compiler source.

I think this is the last PR in this chain, for now. See #12062 for further cleanup (waiting for other unit tests to be completed first).

Relates-to: #10866
Fixes: #12047

@keymanapp-test-bot skip

…VerifyKeyboardVersion()`

Remove the `VERIFY_KEYBOARD_VERSION()` and
`VERIFY_KEYBOARD_VERSION_ret()` macros, and instead use
`VerifyKeyboardVersion()`. The DRYing out of this function call pattern
moved the return statement into a macro, which is an inappropriate way
to reduce repetition, because (a) it is hard for devs to spot function
exits, and (b) automated code analysis tools stumble on it.

Also means that we uncover more clearly the return mismatches between
`KMX_DWORD` and `KMX_BOOL` through the compiler source.

Fixes: #12047
@mcdurdin mcdurdin requested a review from darcywong00 as a code owner July 30, 2024 06:24
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Jul 30, 2024

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

@darcywong00 darcywong00 modified the milestones: A18S7, A18S8 Aug 2, 2024
Base automatically changed from chore/developer/support-message-filename-kmcmplib to master August 3, 2024 10:57
@mcdurdin mcdurdin merged commit 0bd6990 into master Aug 3, 2024
5 checks passed
@mcdurdin mcdurdin deleted the refactor/developer/12047-remove-VERIFY_KEYBOARD_VERSION branch August 3, 2024 11:02
@keyman-server
Copy link
Collaborator

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

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.

refactor(developer): remove return flow control from VERIFY_KEYBOARD_VERSION macro in kmcmplib
4 participants