can we have id property included for PullRequest?

[YunLi1988]

the json response includes the id not for PullRequest object, Can we have this id implemented? Thanks a lot.

[shiftkey]

The Id field isn't used in any of the APIs (Number is the recommended identifier) but for the sake of completeness if someone wants to add in the field this is the response model.

[ryangribble]

@YunLi1988 can you share your use case? My concern is that by adding this Id field which as @shiftkey says isnt actually used for any other API calls, it will just lead to confusion with users using Id instead of Number

[YunLi1988]

Thanks @shiftkey @ryangribble for the quick response! My usage scenario is when integrating pullrequests for different repos, the numbers are no longer identical. For now I can use repo + number as the ID to differentiate them. But it would be super helpful for me to use the official id unified by github. Thanks again!

[ryangribble]

OK no worries, we can just make sure that the XmlDoc comments on this Id field are sufficiently explanatory to hopefully avoid users mistakenly using it when they really want pullrequest.Number

Are you going to have a crack at adding this field to PullRequest class @YunLi1988 ? Happy to guide you through it! It's pretty much just a case of adding the field to the class (and including it in the constructor) then ensuring we check that field is returned in an (existing) integration test

[YunLi1988]

I would like to help and please let me know more.

[ryangribble]

Awesome! Well basically you just need to add the new field long Id to the PullRequest response class here and also include it in the constructor here. You can look at all the existing fields of that response class to get an idea of what is required.

The octokit deserializer will take care of populating the field etc, so the only other thing to do would be to update an integration test (for example this one here) and add an assertion that the new field is populated correctly - eg `Assert.NotNull(result.Id);

Have a crack (fork the repo, create a branch, make some changes) then raise a pending Pull Request here, and we can work through any changes there! Good luck 😀

[YunLi1988]

I have send out this pull request for review. #1537

Thanks a lot for your guidance.

[ryangribble]

Fixed by #1537