Fix Buddy.works isPR because PR ids are not always ints #917
Conversation
| @@ -61,7 +61,7 @@ describe(".isPR", () => { | |||
| describe(".pullRequestID", () => { | |||
| it("pulls it out of the env", () => { | |||
| const buddyWorks = new BuddyWorks(correctEnv) | |||
| expect(buddyWorks.pullRequestID).toEqual("1799") | |||
| expect(buddyWorks.pullRequestID).toEqual("pull/1799") | |||
There was a problem hiding this comment.
This will need you to return the number "1799" - so I'd check maybe for "pull" in the string and extract the number out in the BuddyWorks class 👍
There was a problem hiding this comment.
I wasn't aware of that, it seemed to be working just fine. I'm almost certain there's another env variable which holds just the number. I'll update the PR in a few hours @orta
Thanks for review!
There was a problem hiding this comment.
BTW I was wondering if it made sense to change the return type of get pullRequestID to number? :) This way it would be clear just by looking at the definition that a number is required.
There was a problem hiding this comment.
Maybe it was being set from somewhere else? The GitHub API takes strings for the pull ID - likely legacy on their side, but I'm open to making them numbest so long as it's parsed back into a string in the API layers
|
Cool, thanks! |
|
Thanks for the PR @mmiszy. This PR has been shipped in v9.2.1 - CHANGELOG. |
I was testing Buddy.works CI and it seems the integration with Danger doesn't work. I figured it's because of an incorrect assumption that PR ids are always integers. In my case, in a public GitHub repository, PR id is eg.
pull/42.I tested this and it works fine now 👍
Pinging @kristof0425 because he implemented the integration.