#458: render both between bounds#489
Conversation
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Reviewed the two-file diff against #458. The between bylaw text now names the value and both bounds, matching the operand order used by BTerm#calc, and the regression covers the rendered bylaw string.
Validation I ran on bbfd7aaf3b525167f9853fa42ee104863954509b:
env BUNDLE_PATH=/private/tmp/bundle-fbe-488 mise x ruby@3.3.11 -- bundle exec ruby -Itest test/fbe/test_award.rb -- --no-cov-> 8 tests, 43 assertions, 0 failures, 0 errors.env BUNDLE_PATH=/private/tmp/bundle-fbe-488 mise x ruby@3.3.11 -- bundle exec rake test-> 284 tests, 804 assertions, 0 failures, 0 errors, 9 skips.env BUNDLE_PATH=/private/tmp/bundle-fbe-488 mise x ruby@3.3.11 -- bundle exec rubocop lib/fbe/award.rb test/fbe/test_award.rb --format simple-> 2 files inspected, no offenses detected.env BUNDLE_PATH=/private/tmp/bundle-fbe-488 mise x ruby@3.3.11 -- ruby -c lib/fbe/award.rband... -- ruby -c test/fbe/test_award.rb-> Syntax OK.git diff --check origin/master...HEAD-> passed.git diff origin/master...HEAD | gitleaks stdin --no-banner --redact --exit-code 1-> no leaks found.
No changes requested.
|
@GHX5T-SOL Thanks for the review! You've earned +6 points for this contribution: +18 as a basis, -8 for absolutely no comments posted, and -4 for fewer than 24 hits-of-code (7 total). Your running score is +134; don't forget to check your Zerocracy account too. |
|
@akmhatey-ai merge conflicts here |
|
Please fix the merge conflicts so this pull request can be merged. |
morphqdd
left a comment
There was a problem hiding this comment.
Correct fix for #458 — between has three operands (value, min, max), but the old text rendered at least operands[0] and at most operands[1], dropping operands[2] and mislabeling the value as a bound. The new X clamped between MIN and MAX renders all three correctly, and the test pins the markdown. LGTM.
| "minimum of #{to_p(@operands[0])} and #{to_p(@operands[1])}" | ||
| when :between | ||
| "at least #{to_p(@operands[0])} and at most #{to_p(@operands[1])}" | ||
| "#{to_p(@operands[0])} clamped between #{to_p(@operands[1])} and #{to_p(@operands[2])}" |
There was a problem hiding this comment.
Reads correctly now: for (between b 3 120) it renders b clamped between 3 and 120 instead of the previous nonsensical at least b and at most 3. Good catch on the dropped third operand.
|
@morphqdd Thanks for the review! You've earned +9 points for this: +18 as a basis; -5 for very few (1) comments; -4 for too few (7) hits-of-code. Your running score is +756; don't forget to check your Zerocracy account too). |
|
@akmhatey-ai merge conflict here |
Fixes #458.
The bylaw printer for
betweentreated the clamped value as the lower bound and omitted the upper bound. This changes the rendering to describe the value and both bounds:_b_ clamped between **3** and **120**.Currentness and duplicate checks:
:betweendrops the upper bound and mislabels the value as a bound #458 is open, labeledbugandhelp wanted, and unassigned.gh pr list --repo zerocracy/fbe --state all --search "458 OR between upper bound PTerm Award"returned no PRs.PTerm betweenandclamped betweenreturned no PRs.:betweendrops the upper bound and mislabels the value as a bound #458.Validation:
set _b_ to at least _b_ and at most **3**.ruby -e "gem 'minitest', '6.0.6'; gem 'minitest-mock', '5.27.0'; load 'test/fbe/test_award.rb'"passed: 8 tests, 43 assertions.rubocop lib/fbe/award.rb test/fbe/test_award.rbpassed with no offenses.ruby -c lib/fbe/award.rb; ruby -c test/fbe/test_award.rbpassed.git diff --checkandgit diff --cached --checkpassed.git diff --cached | gitleaks stdin --no-banner --redactreported no leaks.Limitations:
bundle exec rake test TEST=test/fbe/test_award.rbis locally blocked on this Windows checkout because Bundler cannot find locked gemsrdoc-7.2.0,psych-5.3.1, andparallel-2.0.1, and the environment also lacksgrepon PATH. The focused direct Ruby test above was run instead.