Change "IssueId" to `long`

octokit / octokit.net

A GitHub API client library for .NET

https://octokitnet.readthedocs.io/en/latest/

MIT License

2.62k stars 1.07k forks source link

Change "IssueId" to `long` #2890

Closed Leh2 closed 2 months ago

Leh2 commented 2 months ago

Resolves #2889

Before the change?

New issues couldn't be handled by the client because GitHub IDs have surpassed int.Max.

After the change?

New issues know works in the client

Pull request checklist

[x] Tests for the changes have been added (for bug fixes / features)
[x] Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Consumers using the "Id" will experience a breaking change and need to update their type to long.

Please see our docs on breaking changes to help!

[x] Yes
[ ] No

JimSuplizio commented 2 months ago

Crazy question, related to the PR after it gets merged. What's the timeline between this getting merged and a release being up on nuget?

Leh2 commented 2 months ago

Unfortunately, there's a lot more that needs to be done than just changing the Id in Issue.cs. For example, the IIssuesClient.cs uses int all over the place for the Issue number (Id). Further, does the GitHub API even support long when you're updating an issue? As far as I can tell, the Latest Rest API version, which is what Octokit is using, only documents Int, not long.

Issue number is not the same as issue id.

JimSuplizio commented 2 months ago

Issue number is not the same as issue id.

You are correct, I am in error.

danmoseley commented 2 months ago

are there any other values that will roll over in the near future and should change to long now as well?

kfcampbell commented 2 months ago

Thank you for the quick fix! Please report back if something seems amiss.

Leh2 commented 2 months ago

Thank you for the quick fix! Please report back if something seems amiss.

Just tested the new package, and everything works as expected again 👍 Thank you for the NuGet release 💪