Adds support for posting statuses #280
Conversation
orta
force-pushed
the
status
branch
4 times, most recently
from
6010047
to
f2e38b0
Compare
source/runner/Executor.ts
Outdated
|
|
||
| const messageForResults = (results: DangerResults) => { | ||
| if (results.fails.length && results.warnings.length) { | ||
| return `All green. ${compliment()}` |
source/runner/Executor.ts
Outdated
|
|
||
| let message = "⚠️ " | ||
| if (results.fails.length) { | ||
| message += pluralize("Errors", results.fails) |
There was a problem hiding this comment.
Possible bug (if I'm reading this function correctly): when there are 0 errors, pluralize() will return "0 Errors"; when there is more than 1 error, pluralize() will return "n Errorss" with two s's.
Looking at some example success and failure statuses I see on GitHub:
Thoughts on stating "
source/platforms/platform.ts
Outdated
| @@ -43,20 +43,10 @@ export interface Platform { | |||
| editMainComment: (newComment: string) => Promise<any> | |||
| /** Replace the main Danger comment */ | |||
| updateOrCreateComment: (newComment: string) => Promise<any> | |||
| /** Fails the current PR */ | |||
There was a problem hiding this comment.
Thoughts on changing this doc comment to something like "Sets the current PR's status. See the GitHub Statuses API documentation." to document that this could either set the PR status as success or failure?
|
So the Danger token on danger doesn't support the status API, but I trust that it's OK |
|
ill try write some tests tomorrrow |
|
Tomorrow comes today - @macklinu if you're up to it, you're welcome to merge + ship this as a release, I'll update the Artsy danger instance to verify it all etc in the UK morning |
|
Yeah I can release a minor update this afternoon/evening - will test at my work as well. I'm still not super clear on how you ship updates, like this commit - d8d1326. Tried reading through #117 but I'm still not 100% on it:
Thanks @orta 😄 |
|
No worries, yeah, I make those commits then push a tag - that's it |
|
The tag triggers a deploy on travis, I'm still new to this idea of not deploying locally - in ruby I make these PRs, danger/danger#836 then do |
|
I wanna try this semanticc versioning biz, but not got futher |
|
That's funny; I actually like deploying from CI more than I do locally. 😄 But I'm going to merge this and ship 0.21.0 using the current deployment process. Thanks for clarifying! |
Re: #279
This has no tests, but shouldn't be too hard to add them pretty soon.