Skip to content

Fix check.bety.version unknown-migration check - #4155

Open
madhavzanwar wants to merge 2 commits into
PecanProject:developfrom
madhavzanwar:GH-TBD-check-bety-version-unknown-migrations
Open

madhavzanwar wants to merge 2 commits into
PecanProject:developfrom
madhavzanwar:GH-TBD-check-bety-version-unknown-migrations

Conversation

@madhavzanwar

Copy link
Copy Markdown
Contributor

Fixed the migration version check to avoid using any() on character vectors containing unknown migration versions.
Previously, this caused a coercion warning followed by a missing value where TRUE/FALSE needed error instead of the expected warning.
The logic now checks the length of the migration version vector directly, allowing unknown versions to be handled safely.
Added tests to verify the correct warning behavior and ensure the function does not fail unexpectedly.

any() was called on a character vector of unknown migration versions, which
raised a coercion warning and then failed with 'missing value where
TRUE/FALSE needed' instead of emitting the intended warning. Test the
length of the vector instead, and add tests.
Copilot AI balanced review requested due to automatic review settings October 12, 2026 22:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@madhavzanwar

Copy link
Copy Markdown
Contributor Author

Hi @infotroph, could you review this when you have a moment? It fixes #4165:
check.bety.version() called any() on a character vector, so a database newer than
the package raised a coercion error instead of the intended warning. Tests are
included and fail without the fix.

The workflows on this fork PR are showing "Action required", so CI hasn't run yet.
Could a maintainer approve them? Thanks!

@madhavzanwar

madhavzanwar commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor Author

@infotroph same for this one: a small fix for #4165 (check.bety.version errored instead of warning about unknown database migrations), with a test.

Resolve the base/settings/NEWS.md conflict by keeping both entries.
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.

2 participants