-
Notifications
You must be signed in to change notification settings - Fork 53
fix(bigshot.lic): v5.11.4 new command checks #2185
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
base: master
Are you sure you want to change the base?
Conversation
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.
Important
Looks good to me! 👍
Reviewed everything up to d3417ac in 1 minute and 56 seconds. Click for details.
- Reviewed
85lines of code in1files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. scripts/bigshot.lic:8
- Draft comment:
Version updated to 5.11.4. Confirm that the version bump aligns with dependency requirements. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. scripts/bigshot.lic:2568
- Draft comment:
New command names 'essence' and 'splashy' have been added to the command modifier regex. Consider reviewing the ordering for readability. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. scripts/bigshot.lic:2570
- Draft comment:
Ensure that Resources.shadow_essence is defined and returns a numeric value before performing comparisons. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. scripts/bigshot.lic:3580
- Draft comment:
For the splashy check, ensure Room.current is not nil to avoid possible runtime errors when accessing its tags. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. scripts/bigshot.lic:21
- Draft comment:
Typo found: There is no space between "splashy" and "(wet)" in the phrase "Rooms that are splashy(wet)". Consider adding a space for improved readability. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment is about a very minor formatting issue in a changelog entry - adding a space before a parenthetical. While technically correct, this falls under "obvious or unimportant" comments that should be removed according to the rules. Changelog entries are informal documentation, not code, and this level of nitpicking about spacing in a changelog is not actionable or important. The meaning is completely clear without the space. This is the kind of trivial comment that wastes the PR author's time. Could this be part of a style guide for the project that requires specific formatting in changelog entries? Perhaps consistency in changelog formatting is important for automated parsing or documentation generation. Even if there were a style guide, this is such a minor issue that it would likely be caught by automated linting if it mattered. There's no evidence of strict formatting requirements for changelog entries in this codebase. The comment is too trivial to be worth the PR author's attention. This comment should be deleted. It's an unimportant, trivial formatting suggestion about a changelog entry that doesn't affect code functionality. It falls clearly under the rule about not making "obvious or unimportant" comments.
Workflow ID: wflow_o3dGRz5W5jDpfSqH
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
@ellipsis review again |
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.
Caution
Changes requested ❌
Reviewed everything up to 83db8e2 in 2 minutes and 49 seconds. Click for details.
- Reviewed
172lines of code in1files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. scripts/bigshot.lic:8
- Draft comment:
Metadata updated: version bumped to 5.11.4 and Lich requirement raised to 5.13.0. Ensure these changes are compatible with all dependencies. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about dependency version changes and asks the author to ensure compatibility, which violates the rules against commenting on dependency changes and asking the author to ensure compatibility.
2. scripts/bigshot.lic:2569
- Draft comment:
The @COMMAND_MODIFIER_REGEX now includes many command names (including 'essence' and 'splashy'). Consider extracting these keywords into an array to improve readability and ease maintenance. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. scripts/bigshot.lic:2575
- Draft comment:
New 'essence' check added to @COMMAND_AMOUNT_CHECKS. Verify that Resources.shadow_essence returns a numeric value and that the comparison (< vs. >=) is consistent with other checks. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to verify the behavior of a new check added to a command. It suggests ensuring that a function returns a numeric value and that the comparison logic is consistent with other checks. This is a request for verification, which is not allowed according to the rules.
4. scripts/bigshot.lic:3576
- Draft comment:
A new 'splashy' room command check has been added. If more room tag checks are needed in future, consider abstracting tag checking into a helper function. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. scripts/bigshot.lic:21
- Draft comment:
Line 21: Consider adding a space before the opening parenthesis in "splashy(wet)" for clarity (e.g., "splashy (wet)"). - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment is about formatting in a changelog entry, not about actual code changes. The rules state "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." A spacing suggestion in a changelog entry is purely stylistic and doesn't affect functionality. This is an extremely minor formatting preference that doesn't warrant a review comment. The changelog is documentation, not code, and the current format is perfectly readable. This violates the rule about not making obvious or unimportant comments. Could this be considered a documentation quality improvement? Some teams do care about consistent formatting in changelogs. However, the rules explicitly say not to comment unless there's a clear code change required, and this is just a style preference. Even if some teams care about changelog formatting, this is an extremely minor stylistic preference with no functional impact. The rules are clear that comments should only be made when there's a clear code change required, and this doesn't meet that threshold. This is the definition of an "obvious or unimportant" comment. This comment should be deleted. It's a minor stylistic suggestion about spacing in a changelog entry, not a code change. It violates the rules about only commenting when there's clearly a code change required and not making obvious or unimportant comments.
Workflow ID: wflow_DdTMo6rgDHSjTHcJ
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Important
Update
bigshot.licto v5.11.4 with new 'splashy' and 'essence' command checks and improved group handling.bigshot.licto version 5.11.4, requiring Lich >= 5.13.0.!?splashyand!?essenceto@COMMAND_MODIFIER_REGEXininitialize_command_data.essencechecks to@COMMAND_AMOUNT_CHECKS.Bigshotclass.This description was created by
for 83db8e2. You can customize this summary. It will automatically update as commits are pushed.