danger / danger-js

⚠️ Stop saying "you forgot to …" in code review
http://danger.systems/js/
MIT License
5.22k stars 363 forks source link

[BUG] GitHubAPIPR unsupported structure #1415

Closed radimsv closed 7 months ago

radimsv commented 7 months ago

Describe the bug GitHubAPIPR contains an attribute number but it is no longer supported for the functions used for REST API calls related to PRs. pull_number is specified in the Github docs https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28

To Reproduce try to make a REST API call, e.g.

danger.github.api.pulls.update({
  ...danger.github.thisPR,
  pull_number: danger.github.thisPR.number, // <-- the annoying part
  title: 'new title'
});

(It works fine^^ when passing both number and pull_number props)

Expected behavior

danger.github.api.pulls.update({
  ...danger.github.thisPR,
  title: 'new title'
});

Your Environment

software version
danger.js 11.3.0 (latest)

Possible Solution PR that adds the pull_number into danger.github.thisPR or replaces the number prop which could be a breaking change for some. This is a decision that should be confirmed by maintainers. I'm fine with creating the PR :)

orta commented 7 months ago

I think if we add pull_number than it won't be a breaking version change - if that's enough to make it work fine with the new API, we should go with that (as its not a breaking change)

fbartho commented 7 months ago

I support adding pull_number, and marking number @deprecated asap.

I probably have a ton of code that needs to adapt to this, but I think it's at places I don't work anymore, haha