-
Notifications
You must be signed in to change notification settings - Fork 9
Fix conflict detection logic #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
It's basically 3 PRs in one, review each commit differently |
const milestoneToPr = new Map<string, number[]>(); | ||
const milestoneToPr = new Map< | ||
string, | ||
{ number: number; user: { login: string } }[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about name
instead of login
?
I had to look below to find out that you mean the name, and not some type of secret or authentication method…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah wait, does it come like that from the GitHub API?
If yes, ignore my comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If no, then why do you not assign the user manually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's from the GitHub API
Regression from #53 Signed-off-by: Yarden Shoham <[email protected]>
Regression from #53 Signed-off-by: Yarden Shoham <[email protected]>
break
), fixes Wrong remind about conflicts #52