Skip to content
This repository has been archived by the owner on Nov 8, 2018. It is now read-only.

post a comment when the 'comment' param exists #24

Closed
wants to merge 2 commits into from

Conversation

fmy
Copy link
Contributor

@fmy fmy commented Jul 25, 2016

Like other CI tools, posting a comment into the pull request.
2016-07-25 16 14 46

  • new param is added: comment which is the file path of the comment message.
    • if this param is passed, try to post a comment with file content.
  • this comment is posted by the user who owns the access_token.

@jtarchie
Copy link
Owner

Thanks for the PR. This is an interesting request.

The statuses on the SHA already return success, failure, etc. The follow up meta comment can be confusing as it is not directly tied to a SHA.

Are you able to give me a use case of why this would be useful? Maybe link to another CI system that does this.

Thanks,

JT

@fmy
Copy link
Contributor Author

fmy commented Jul 26, 2016

@jtarchie My pipeline is creating app instances for testing just after pull request is coming.
In my case, concourse notifies this testing app URLs to the pull request.

And some other use cases listed below

  • post test report URLs.
  • post whether test coverage percentage becomes up or down.
  • post error summaries of test results.

@jtarchie
Copy link
Owner

jtarchie commented Aug 9, 2016

@fmy, I like it. I'm currently doing a bit of rewrite, to make it easier to test. I'll pull this in and backport some tests.

@jtarchie
Copy link
Owner

jtarchie commented Sep 9, 2016

I did merge in your commits. I just did a rebase so this PR does not reflect the updated SHAs.

Thanks for the PR

@jtarchie jtarchie closed this Sep 9, 2016
@fmy
Copy link
Contributor Author

fmy commented Sep 9, 2016

Thanks!

@fmy fmy deleted the put-comment branch September 9, 2016 02:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants