Conversation
gwynne
left a comment
There was a problem hiding this comment.
Per inline comments. You're gonna hate me for this one.
|
There is a known warning in |
gwynne
left a comment
There was a problem hiding this comment.
Some of these are tiny nits, others are serious concerns. At least two are actual logic errors, and there's a major threading issue to boot. (At least, assuming I didn't misread anything or do the logic in my head wrong, we all know that happens a lot 😆)
|
@MahdiBM Watch out for Github's tendency to hide comments behind "5 hidden conversations" etc. links on the Conversation page of PRs; I miss comments that way more often than I care to admit 🤦♀️. |
Co-authored-by: Gwynne Raskind <gwynne@vapor.codes>
Co-authored-by: Gwynne Raskind <gwynne@vapor.codes>
gwynne
left a comment
There was a problem hiding this comment.
Almost there, I think 😰 Sorry that I ended up worrying about threading stuff anyway after all that - I'm considerably less confident of my conclusions this time, but it nonetheless seems correct to me after careful reading of the linked info. Please do feel free to challenge my reasoning if you see any (more) mistakes 😅
gwynne
left a comment
There was a problem hiding this comment.
Okay, finally LGTM 😅 Excellent work!
Check list
mergedistrueand detect the semver-patch/minor/major labels.latesttag. Don't just doreleases.last.