Don't output table if no violations are specified #116
Conversation
Right now when using `fail` just to fail the build, but not to output any information (i.e. just using `fail()`), Danger outputs an empty table with a message of "undefined", which is annoying. (see danger#109 for context) I _think_ this fixes it, but I'm not sure if it's the right fix and/or if there's a better way to do it. Please let me know!
| @@ -11,7 +11,7 @@ import * as v from "voca" | |||
| * @returns {string} HTML | |||
| */ | |||
| function table(name: string, emoji: string, violations: Array<Violation>): string { | |||
| if (violations.length === 0) { return "" } | |||
| if (violations.length === 0 || (violations.length === 1 && !violations[0].message)) { return "" } | |||
There was a problem hiding this comment.
Lets get a test on this logic, as this is not something which would get manually tested very often
There was a problem hiding this comment.
This area is already pretty well covered, so it should be pretty trivial to C&P an existing one
| @@ -2,11 +2,13 @@ | |||
|
|
|||
| // Add your own contribution below | |||
|
|
|||
| * Add support for failing the build without outputting any text - mxstbr | |||
There was a problem hiding this comment.
This should probably be something more like:
Builds which only use
markdownnow only show the markdown, and no violations table is shown - mxstbr
Which makes a bit more sense without the context of this PR 👍
|
I just realised that this would still show a table in this case: fail()
fail()Which doesn't make sense. Will update, one sec. |
|
@mxstbr haha I was just about to comment on that potential edge case - nice catch! 😄 |
|
Done, take a look and let me know! |
|
@macklinu I'd love to give you permission to push |
| @@ -11,7 +11,7 @@ import * as v from "voca" | |||
| * @returns {string} HTML | |||
| */ | |||
| function table(name: string, emoji: string, violations: Array<Violation>): string { | |||
| if (violations.length === 0) { return "" } | |||
| if (violations.length === 0 || violations.every(violation => !violation.message)) { return "" } | |||
There was a problem hiding this comment.
cool, the every function is new to me
There was a problem hiding this comment.
Yeah, I've only known it for a few weeks! (there's also some FYI)
|
My rule of thumb on releasing is roughly every 2-3 PRs, I have zero problems with every PR, assuming every PR is big enough to warrant the ~2 minutes to deploy |
|
|
That sounds like the perfect use case for |
Right now when using
failjust to fail the build, but not to outputany information (i.e. just using
fail()), Danger outputs an emptytable with a message of "undefined", which is annoying. (see #109 for
context)
I think this fixes it, but I'm not sure if it's the right fix and/or
if there's a better way to do it. Please let me know!