WIP: Change all applicable IDs in the project from int --> long

[SeanKilleen]

It's been a long time coming. 😆

Resolves #2351.

⚠I assume this would mandate a breaking/major release.

My Approach

If I'm going to be honest, it's a little bit "search and destroy". I'm going for wholesale change in the project and ensuring that all IDs can be longs going forward. So I'm searching with a regex of int [^=]*Id and int\? [^=]*Id and going from there.

Work

• Find ID values
• Change said ID values to longs
• Do it everywhere
• Make sure the tests pass
• Double check int [^=]*Id search
• Double check int\? [^=]*Id
• Do a search for int [^=]*number just in case
• Do a search for int\? [^=]*number just in case
• Ensure that that the int IDs I accidentally captured as long have been changed back.

Items To Confirm / Change based on feedback

• ❌ PullRequestReview.Number Seems like it should remain int, given it's the display value and I don't think we're in danger of running out there) leaving alone for now per comments.
• ❌ Account.DiskUsage should this remain an int? Need to double-check on how it's used. leaving alone per comments
• ❌ GpgKey.PrimaryKeyId this is an int?. Not sure enough of the usage to change it right away Treating like an ID but ensuring it handles null. (Update: leaving this alone as API defines it a an int.)

[shiftkey]

⚠I assume this would mandate a breaking/major release.

I worry about updating to 1.0 but am happy to do a big breaking changes note for this release.

[SeanKilleen]

@shiftkey actually I think given the weight of 1.0 release and the current availability of maintainers, I'd keep it as the next 0.x and publicize the changes. We can make sure there's a good write-up in the release notes.

[SeanKilleen]

An update here -- it's clear at this point from the many "oops" commits that I misunderstood that many IDs in the API level are indeed intended to be ints even if I might expect them to exceed that boundary at some point. 😅

So, my next steps to get through this are:

• Make the branch work, tests and all, with the changes I've already made
• Go back through every change and ensure it is in line with the API v3 specifications, and correct where it's not
• Once again ensure everything is building and passing.

At that point we should be back on the right track.

[SeanKilleen]

Sigh....this is on me. @shiftkey, the more I look at it, the more these IDs are correctly ints in the API docs and my mapping them to longs looks incorrect. I think I should have done more research before jumping into this. But I am not deterred. 😄

We have a few options:

• Revert any IDs that aren't explicitly longs in the API docs
• Convert IDs to longs regardless of API docs, for graceful upgrades in the future, given that since we're reading there's little chance of an issue cropping up.

I'm fine to proceed either way, including if I should close this out and start over to get rid of the mess. Let me know and I'll get it all wrapped up the way we want. 👍

[SeanKilleen]

@shiftkey what do you think?

1. Match the return types of the API (e.g. leave int if API specifies integer)
2. Convert all IDs to long regardless of API type
3. Convert all IDs -- and numbers -- to long regardless of API type

Happy to see this through, but realized I should probably get that clarification first.

[github-actions]

👋 Hey Friends, this pull request has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!