Skip to content

Conversation

@RMacfarlane
Copy link
Contributor

@RMacfarlane RMacfarlane commented Sep 26, 2020

Fixes #2136

isIssue: false
isIssue: false,
isAuthor: currentUser.login === pullRequest.author.login,
reviewers: this.parseReviewers(requestedReviewers || [], timelineEvents || [], pullRequest.author)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have ?? should we be using that to set default values instead of or? I don't feel super strongly, just curious what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I realize I didn't actually respond to this! I don't have strong opinions about this syntax, but I like it and think it's worth adopting

* @param timelineEvents All timeline events for the pull request
* @param author The author of the pull request
*/
private parseReviewers(requestedReviewers: IAccount[], timelineEvents: TimelineEvent[], author: IAccount): ReviewState[] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate method from pullRequestOverview.ts?

@RMacfarlane RMacfarlane merged commit 38e7347 into master Oct 14, 2020
@RMacfarlane RMacfarlane deleted the rmacfarlane/webviewImprovements branch October 14, 2020 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PR overview in sidebar

3 participants