-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Value was either too large or too small for an Int32 #598
Comments
Upgrading Octokit is a good first step but it's not sufficient to resolve this issue. I was able to reproduce this problem even after upgrading the reference. Here's a more detailed stack trace:
I notice that stack trace mentions public class IssueComment
{
/// <summary>
/// Gets or sets the issue comment Id.
/// </summary>
public int Id { get; set; } Probably should change the type of the I scanned all the model classes and noticed a few more properties currently defined as
|
After further investigation it appears that Octokit still defines |
I can confirm that after upgrading the Octokit reference and changing
|
Submitted a PR with necessary changes. I did not change Milestone.InternalNumber to long since it's not needed to resolve this issue but I'll be happy to amend my PR if the consensus is that we should proactively change it as well. |
@Jericho thank you for working on this! Appreciate the help (again)! I have marked this issue as a breaking change, as the required change will need to modify some of the internals. Rather than stray too far from the underlying dependencies, I think we should wait before making any additional changes. Reading the issue that you linked to, it would appear that a new Octokit library is in the works, that is generated from the underlying API, so issues like this shouldn't happen going further, so perhaps better to wait until that is ready, and then switch over to using that. |
To addresses issues with int overflows. This was fixed in Octokit by changing int to long, in some places. In addition, changes IssueComment.Id to long to match the underlying type.
* release/0.18.0: (#580) Mark the integration test as "inconclusive" (test) Remove usage of Assert.AreEqual Bump NUnit from 3.14.0 to 4.1.0 in /src (test) Introduce NUnit.Analyzers Bump NGitLab from 6.39.0 to 6.51.1 in /src Bump Serilog.Sinks.Console and Serilog in /src Bump Microsoft.Extensions.DependencyInjection in /src Bump Microsoft.SourceLink.GitHub from 1.1.1 to 8.0.0 in /src Bump ApprovalTests from 5.9.0 to 6.0.0 in /src Bump Scriban from 5.9.0 to 5.10.0 in /src Bump Destructurama.Attributed and Serilog in /src Bump Microsoft.CodeAnalysis.NetAnalyzers from 7.0.4 to 8.0.0 in /src Bump Roslynator.Analyzers from 4.6.2 to 4.12.4 in /src (build) Skip running Codecov (#598) Upgrade Octokit reference
Merge branch 'release/0.18.0' * release/0.18.0: (#580) Mark the integration test as "inconclusive" (test) Remove usage of Assert.AreEqual Bump NUnit from 3.14.0 to 4.1.0 in /src (test) Introduce NUnit.Analyzers Bump NGitLab from 6.39.0 to 6.51.1 in /src Bump Serilog.Sinks.Console and Serilog in /src Bump Microsoft.Extensions.DependencyInjection in /src Bump Microsoft.SourceLink.GitHub from 1.1.1 to 8.0.0 in /src Bump ApprovalTests from 5.9.0 to 6.0.0 in /src Bump Scriban from 5.9.0 to 5.10.0 in /src Bump Destructurama.Attributed and Serilog in /src Bump Microsoft.CodeAnalysis.NetAnalyzers from 7.0.4 to 8.0.0 in /src Bump Roslynator.Analyzers from 4.6.2 to 4.12.4 in /src (build) Skip running Codecov (#598) Upgrade Octokit reference
🎉 This issue has been resolved in version 0.18.0 🎉 The release is available on: Your GitReleaseManager bot 📦🚀 |
Merge branch 'release/0.18.0' * release/0.18.0: (#580) Mark the integration test as "inconclusive" (test) Remove usage of Assert.AreEqual Bump NUnit from 3.14.0 to 4.1.0 in /src (test) Introduce NUnit.Analyzers Bump NGitLab from 6.39.0 to 6.51.1 in /src Bump Serilog.Sinks.Console and Serilog in /src Bump Microsoft.Extensions.DependencyInjection in /src Bump Microsoft.SourceLink.GitHub from 1.1.1 to 8.0.0 in /src Bump ApprovalTests from 5.9.0 to 6.0.0 in /src Bump Scriban from 5.9.0 to 5.10.0 in /src Bump Destructurama.Attributed and Serilog in /src Bump Microsoft.CodeAnalysis.NetAnalyzers from 7.0.4 to 8.0.0 in /src Bump Roslynator.Analyzers from 4.6.2 to 4.12.4 in /src (build) Skip running Codecov (#598) Upgrade Octokit reference
GRM references Octokit 10.0.0 which has known bugs regarding
int
vslong
. We should upgrade to Octokit 12.0.0 which resolves some of these bugs (such as octokit/octokit.net#2928 for instance).BTW: my guess is that more of these int vs long issues will be discovered and solved in the near future so I wouldn't be surprised if we had to upgrade again in the future.
Here's an example demonstrating that my build is failing due to this issue:
https://ci.appveyor.com/project/Jericho/stronggrid/builds/50054134/job/k3i5irek8a2bsa33#L487
The text was updated successfully, but these errors were encountered: