Skip to content
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

Feature: Upgrade .NET from Core 3.1 to 8.0 (and Native AOT!) #321

Draft
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

Lamparter
Copy link
Contributor

@Lamparter Lamparter commented Sep 7, 2024

Related: #293 (hey there!)
This pull request was authored on behalf of @RiversideValley

Before the change?

  • Dependency: .NET Core 3.1 Runtime

After the change?

  • Dependency: .NET 8.0 Runtime

Pull request checklist

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

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

Yes


Hello! Me and @0x5bfa are really interested in this project as it is used in our project @FluentHub
We'd like to propose a few changes, such as Native AOT (exciting! 🚀 but for another PR) and .NET 8 target runtime support. This PR updates the library/test runs to .NET 8. Not quite complete yet though.

Copy link

github-actions bot commented Sep 7, 2024

👋 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 labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@Lamparter

This comment was marked as resolved.

@mousemaid
Copy link

mousemaid commented Sep 7, 2024

The issue was fixed in e445b1c 💚

To view the logs, click here 😄
   ___  ___    _   ___ _    ___ 
  / _ \| _ \  /_\ / __| |  | __|
 | (_) |   / / _ \ (__| |__| _| 
  \___/|_|_\/_/ \_\___|____|___|
                                
"Connected to Oracle server in tenancy AD-2, South UK"
winget install Git.Git
winget install Microsoft.DotNet.SDK.8
[Riverside].Respire
"Found: Octokit.GraphQL.UnitTests"
"Found: Octokit.GraphQL.IntegrationTests"
"Other projects found, but are .NET Libraries. Will not deploy, but will still provide build results."
[Riverside].ApplicationHostServices
"Build started at 10:12..."
"Build completed at 10:13 and took 1 minutes"
[Riverside].ApplicationHostGateway
"Compiling log"
"Log returned to debug gateway"

Octokit.GraphQL.Core.UnitTests.ConnectionTests.Run_Specifies_Cancellation_Token
📋 Source:ConnectionTests.cs line 60
🕒 Duration:3 ms

System.Threading.Tasks.TaskCanceledException: A task was canceled.
System.Threading.Tasks.TaskCanceledException: A task was canceled.

HttpClient.HandleFailure(Exception e, Boolean telemetryStarted, HttpResponseMessage response, CancellationTokenSource cts, CancellationToken cancellationToken, CancellationTokenSource pendingRequestsCts)
HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
Connection.Run(String query, CancellationToken cancellationToken) line 211
ConnectionTests.Run_Specifies_Cancellation_Token() line 73
HttpContent.LoadIntoBufferAsyncCore(Task serializeToStreamTask, MemoryStream tempBuffer)
HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)

@Lamparter Lamparter changed the title Feature: Upgrade .NET from Core 3.1 to 8.0 Feature: Upgrade .NET from Core 3.1 to 8.0 (and Native AOT!) Sep 7, 2024
@Lamparter

This comment was marked as resolved.

@kfcampbell
Copy link
Member

FWIW I would consider this a breaking change due to the runtime upgrade, though I think it's a good idea given that .NET Core 3.1 was end of life'd in December 2022.

@Lamparter
Copy link
Contributor Author

Lamparter commented Sep 28, 2024

This path no longer exists because of the upgrade, causing an error with the CI
image

Is this part of ${{ env.OG_PACK_PROJ_PATH }}?

@0x5bfa
Copy link
Contributor

0x5bfa commented Nov 8, 2024

@Lamparter I fixed the CI. Could you:

  • Tell me if this is already published AoT and works well?
  • Squash my these 17 commits? (GH Desktop > History > Select them > right click to Squash but you have to rebase first Branch > Rebase > Select origin/main)

@0x5bfa
Copy link
Contributor

0x5bfa commented Nov 8, 2024

FWIW I would consider this a breaking change due to the runtime upgrade, though I think it's a good idea given that .NET Core 3.1 was end of life'd in December 2022.

@kfcampbell Are you wanting to bump the version up here?

Signed-off-by: 0x5BFA <[email protected]>
Signed-off-by: 0x5BFA <[email protected]>
Signed-off-by: 0x5BFA <[email protected]>
…sions.1.12.1.nupkg to src/Octokit.GraphQL.Core.UnitTests/Assets/AgileObjects.ReadableExpressions.1.12.1.nupkg

Signed-off-by: 0x5BFA <[email protected]>
Signed-off-by: 0x5BFA <[email protected]>
@Lamparter
Copy link
Contributor Author

FWIW I would consider this a breaking change due to the runtime upgrade, though I think it's a good idea given that .NET Core 3.1 was end of life'd in December 2022.

[redacted] Are you wanting to bump the version up here?

I already bumped it. They were talking about marking this pr as a breaking change because I'd updated it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

4 participants