Posts the comment URL to the CLI #573
Conversation
|
Works correct: |
source/platforms/BitBucketServer.ts
Outdated
| return true | ||
| // We want to make a URL like: | ||
| // https://bitbucket.org/atlassian/jiraconnect-ios/pull-requests/4/crash-groups/diff#comment-426 | ||
| // But I don't see |
There was a problem hiding this comment.
Did you forget to add the rest of the comment? 😄
There was a problem hiding this comment.
lol yep, took a while to figure out how to make the link
| } | ||
|
|
||
| return true | ||
| return issue && issue.html_url |
There was a problem hiding this comment.
this is a really smart way of avoiding accessing a property on null, although it wasn't very clear to me at first so I had to play with it to confirm that it does what I think it does 😄
There was a problem hiding this comment.
Yeah, this is definitely a JS-ism
| @@ -227,13 +227,14 @@ export class GitHub implements Platform { | |||
| * @param {string} newComment string value of comment | |||
| * @returns {Promise<boolean>} success of posting comment | |||
| */ | |||
| async updateOrCreateComment(dangerID: string, newComment: string): Promise<boolean> { | |||
| async updateOrCreateComment(dangerID: string, newComment: string): Promise<string | undefined> { | |||
There was a problem hiding this comment.
Sorry if obvious, but why is there an undefined type here instead of null?
There was a problem hiding this comment.
There's very little difference between undefined and null, but they are semantically different:x = {} and x = {a:undefined} respond the same to x.a but {a: null} is different. So I use null to define a conscious missing value - vs undefined which is kinda "not there", if you're doing an if(x.a) they're all the same, but some libs will treat null different from undefind
Fixes #572 - also sets the clicky link in the status to be that comment