Github ability to comment on an issue

[trinityXmontoya]

👋 hello!

re: #899

qs:

• it was unclear to me where :result/data is used, and in the case of commenting what should be returned. the url of the comment perhaps? here is response body
• what's the best way for me to test this? didn't see notes in the contributing doc
• why do the cmd regex matchers use \S(any non-whitespace character)? afaik github repo names can contain letters, numbers, underscores, dashes, and periods. i bring this up because the regex parsing for my command (and others) will 'pass' but be incorrect if someone enters multiple "\s" or "#"s. clearly this code has existed for awhile and not been changed so it must not be causing problems but jw :)

ex ->

(def regex #"comment\s+(\S+)\/(\S+)#(\d+)\s+(.+)")

(def good-input  "comment org/repo#1 comment")
(re-find regex good-input) -> ["comment org/repo#1 comment" "org" "repo" "1" "comment"]

(def bad-input "comment org//repo##1 comment")
(re-find regex bad-input)-> ["comment org//repo##1 comment" "org/" "repo#" "1" "comment"]

[devth]

Hi @trinityXmontoya! Thanks for the PR, this is awesome! 💥

it was unclear to me where :result/data is used, and in the case of commenting what should be returned. the url of the comment perhaps? here is response body

I'd return the entire response body as :result/data. This is used by a few commands that can access the raw data behind any command for things like custom formatting. Check out the All your data are belong blog post or the Data and Render sections of the docs for more info on that.

what's the best way for me to test this? didn't see notes in the contributing doc

Yep, I need to document this better. We've been experimenting with some various testing methodologies lately and I haven't documented it yet. You can add tests if you'd like to but I'll take the PR either way :)

I've been testing some commands using a utility function called command-execution-info, and asserting things like the result of regex matching and that a given command is triggering the correct handler function (e.g. comment-on-issue-cmd). Check out the pagerduty tests for an example of what that looks like. We also sometimes mock out the api responses using Midje's provided capability. See the existing github tests for examples using provided.

why do the cmd regex matchers use \S(any non-whitespace character)? afaik github repo names can contain letters, numbers, underscores, dashes, and periods. i bring this up because the regex parsing for my command (and others) will 'pass' but be incorrect if someone enters multiple "\s" or "#"s. clearly this code has existed for awhile and not been changed so it must not be causing problems but jw :)

You're right - this was just me being too lazy to precisely capture the requirements of a github repo name. If you want to lock it down to the proper precise regex that would be a welcome addition!

Thanks!

[devth]

Hey @trinityXmontoya, did you want to make any other changes or is this ready for merge?

[trinityXmontoya]

Hey @devth ! Thanks for your patience, I had reached out to Github support to confirm org + repo naming rules, they got back to me yesterday so I'll be making those changes later today!

Github Developer Support - Naming Rules.pdf

[devth]

Awesome, thanks!