You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR Review: fix(pegboard): create missing normal runner configs
Summary
This is a minimal, correct one-character logic inversion bug fix. The function pegboard_runner_config_ensure_normal_if_missing is supposed to create a default "normal" runner config if one does not already exist. The original code had the condition inverted:
// Before (bug): creates config only when one already existsif !pool_res.is_empty(){ ...}// After (fix): creates config only when none existsif pool_res.is_empty(){ ...}
The broken condition caused the upsert to fire when a config already existed and silently skip creation when one was missing — the exact inverse of the intended semantics described in the function's doc comment.
Feedback
Correctness — Fix is correct. The get call returns a Vec<RunnerConfig> filtered to found entries, so an empty result means the config is absent. The corrected is_empty() guard now matches the documented intent.
Pre-existing doc comment nit (not introduced by this PR): the doc comment fragment Returns true when the config was created is stale since the function returns Result<()>, not Result<bool>. Worth cleaning up.
Test coverage — No tests are added. Given the function is used as a workflow activity in runner2.rs, a regression test confirming that configs are created when absent (and not overwritten when present) would prevent this class of inversion bug from re-appearing.
Assessment
Correct, minimal, and safe. No style/convention violations, no security concerns, no performance impact. The only suggestions are a pre-existing stale doc comment and the absence of a regression test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: