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.
Refactor
engine.max-runsto top-levelmax-runswith AWF enforcement #31418New 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
Uh oh!
There was an error while loading. Please reload this page.
Refactor
engine.max-runsto top-levelmax-runswith AWF enforcement #31418Changes from 2 commits
ac48245e4906c9e971242f1dc62eFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/improve-codebase-architecture] The first loop (lines 38–55) scans lines to extract
maxRunsSuffix, but the value is already available from theengineMappassed in as frontmatter — no need for a second traversal:This removes ~18 lines of stateful scanning and eliminates the subtle edge case where a comment line inside the engine block could confuse the
inEngineBlockstate machine.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/tdd] If
engine:is not found inresult(shouldn't happen in practice, but the loop doesn't guard against it),insertAtstays0andmax-runsis silently prepended to the very start of the frontmatter. This is hard to detect without a dedicated test.Consider adding an early return:
A test with an edge-case input where
engine:has no sub-keys left after removal would cover this path.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/tdd]
assert.Containson the transformed string checks substring presence but doesn't enforce exact structure — if whitespace or another line snuck betweenmax-runs:andengine:, the assertion would fail with a confusing message. Usingassert.Equalon the full result makes the spec explicit:This also doubles as a regression guard against inadvertent whitespace changes.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.