Issue content should not be updated when closing with comment #2833
Conversation
lafriks force-pushed the fix/issue_comment_close branch 2 times, most recently from 5b71fe4 to 5508747 Compare 
| } | ||
| | ||
| func testNewIssue(t *testing.T, session *TestSession, user, repo, title string) { | ||
| func testNewIssue(t *testing.T, session *TestSession, user, repo, title, content string) string { |
There was a problem hiding this comment.
nit: from the signature, it's not obvious what the function returns. Could we add a brief comment specifying that the created issue's URL is returned?
| return issueURL | ||
| } | ||
| | ||
| func testIssueAddComment(t *testing.T, session *TestSession, issueURL, content, status string) { |
There was a problem hiding this comment.
nit: it's not obvious what values are valid for status (without looking through other parts of the codebase), and if I see something like testIssueAddComment(t, sess, url, "hello", "close") in a test, it's not obvious that this will close the issue, in addition to making a comment.
Could we instead do:
func testIssueAddComment(t, session, issueURL, content) { testIssueAndCommentWithStatus(t,session,issueURL,content,"") } func testIssueAddClosingComment(t, session, issueURL, content) { testIssueAndCommentWithStatus(t,session,issueURL,content,"close") } // helper function func testIssueAddCommentWithStatus(t, session, issueURL, content, status) { ... } I think this will make it easier for future tests to reuse these helpers, and will make tests more readable.
| LGTM |
tboerger added the lgtm/need 1 
| But CI failed. |
Codecov Report
@@ Coverage Diff @@ ## master #2833 +/- ## ======================================= Coverage 26.85% 26.85% ======================================= Files 89 89 Lines 17607 17607 ======================================= Hits 4728 4728 Misses 12193 12193 Partials 686 686
Continue to review full report at Codecov.
|
|
|
| LGTM |
tboerger added lgtm/done 
5 participants
Fixes #2551