RFR: 1789: Skara PR commands are not interpreted on Github review comments [v2]
Magnus Ihse Bursie
ihse at openjdk.org
Thu Jan 19 14:44:28 UTC 2023
On Wed, 18 Jan 2023 21:41:03 GMT, Zhao Song <zsong at openjdk.org> wrote:
>> A user reported that Skara bots were not responding to commands in review comments or the body of a review.
>> This patch addresses that issue by ensuring that the Skara bots can correctly detect and interpret commands.
> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
> sort comments by created time
It sounds reasonable to not allow PR comments in review comments on individual lines, yes. As Erik says, they are presented differently, can be hidden, and form a "different kind" of communication.
I still think the main review "comment" (or whatever to call it) deserves to be parsed for PR comments. Once it is posted, it looks just like a regular comment. If it were not for Skara treating this differently, I would have had no idea it was even treated differently internally in Github. To me, it is like a "approve this PR + also add this comment", not "approve this PR with this special kind of comment that can only be generated here but looks like any other normal comment".
> Even in the main review comment what gets scanned - all the text? Only the start of lines? I would worry about text being misconstrued as a command. This would especially be true for skara PRs where review comments may well refer to PR commands as part of the discussion.
I am not sure what you mean by that? My example is the kind of comment I've been doing far too many times before remembering that it will not work, I look at a change, check the build changes, presses the green button, selects "Approve" and writes:
This looks good from a build perspective, but you'll need someone from core-libs to check it as well.
Thank you @openjdk bot for providing a quick example that the risk for "misconstruing" commands are just as present in normal commands, and not something that will be a special problems for main review comments. :-D
More information about the skara-dev