octokit.graphql.net
octokit.graphql.net copied to clipboard
feat: breaking change: revive cleanup projects PR, target .NET 6.0
BREAKING CHANGE: target .NET 6.0.
Builds the project on .NET 8. Also makes the following changes of @0xced's from #304:
- Update test projects from .NET Core 3.1 (out of support) to .NET 6 (LTS)
- Remove
<Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
(see https://github.com/Microsoft/vstest/issues/472#issuecomment-378811616) - Remove
<LangVersion>7.2</LangVersion>
on test projects - Fix
Run_Specifies_Cancellation_Token
which (rightfully) throws when running on .NET 6
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs
. You & others like you are the reason all of this works! So thank you & happy coding! 🚀
https://github.com/octokit/octokit.graphql.net/pull/313/commits/4661572495ec86fd03b540e800cc727a93157b26 has a test "fix" for a serialization issue that occurs when updating .NET versions. Essentially, the expected string query string in tests looks like (note the x => new
piece):
data => Rewritten.Value.Select(
data["data"]["repository"]["issue"],
x => new
{
Value = Rewritten.Value.SingleOrDefault(
Rewritten.Value.Select(
x["value"],
y => new
{
Closed = y["closed"].ToObject<bool>(),
Description = y["description"].ToObject<string>()
}))
})
and the actual string looks like (note the x => new object
piece):
data => Rewritten.Value.Select(
data["data"]["repository"]["issue"],
x => new object
{
Value = Rewritten.Value.SingleOrDefault(
Rewritten.Value.Select(
x["value"],
y => new object
{
Closed = y["closed"].ToObject<bool>(),
Description = y["description"].ToObject<string>()
}))
})
These are the "same" content, but fail string comparisons. I'm not sure how to monkey with our string comparison library or our query generation to get these to be truly identical. @StanleyGoldman, since you originally introduced this comparison mechanism years ago, I'd be very interested to know if you have thoughts!
In the meantime, my hacky "fix" is a find/replace that looks for instances of "new{" in the expected string and replaces them with "newobject{" to match the actual strings. I don't love this approach, hence I'm leaving this PR in draft for the time being.
@nickfloyd and @0xced, I'm also interested to hear if you have comments on this.