-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[BUG]: Octokit.net is not trim compatible #2737
Comments
I'd like to work on this, leaving a comment to make everyone aware that I'm going to prepare a pull request. It might be rather large, as it will take a lot of effort to enable the library to be AOT trimmable.
|
I hit a snag, I'm realizing that the Some problematic APIs: This will allow us to support trimming when building for |
My use case for wanting this library to be trimmable, is that I plan on using it in several GitHub Action .NET container apps, where startup time is greatly improved when using AOT. This is vital to the success of the adoption of these apps. Right, so after digging into this - it's going to take a huge amount of time / effort. Here's my findings, summarized as succinctly as possible:
I was describing this situation to a co-worker and they asked what the alternative is to this approach, I don't think there's much that can be done otherwise besides a complete rewrite - which seems like way more work. I do have a branch that I've been working on, but I would like to solicit feedback before continuing work: main...IEvangelist:octokit.net:trimming-aot |
+1 to the ask for support for native AoT here (and in octokit/webhooks.net) - I have a number of apps I use both libraries in, but those are currently blocked from using AoT because I use parts of ASP.NET Core that aren't yet supported, such as Razor Pages, so Octokit also not supporting is is moot for me...for now. I will be looking to enable AoT in those apps as soon as that blocker goes away (.NET 9? 🤞), so knowing that Octokit supports native AoT in advance of that would be most welcome. |
Hey Friends ❤. This is such a good discussion. Thanks @hach-que for kicking it off. Just as y'all know / discovered we have some old patterns, implementations, and code (that use reflection specifically) that make converting octokit.net to be able to benefit from AoT almost impossible. Bad news out of the way, here's the good news... 🎉 We are in the middle of a seriously exciting effort where we are using the incredible work that our Microsoft friends have done (Kiota) and are leaning heavily into generating our SDKs - We alluded to this over a year ago and have continued the discussion here and here). So what does this mean? We'll get AoT support right out of the box with the newly generated dotnet-sdk. What else does this mean? We'll get close to 100% GitHub REST API coverage, have an SDK that supports REST API versioning (lowers the risk of breaking changes), support more Auth patterns, and begin to unlock a host of other things/patterns that we were previously not able to do. Oh and we'll be supporting more languages eventually When is this happening? Hopefully sooner than later. We've been working through some prototyping and are fairly close on getting our thoughts down as code. The goal is to get this things as "right" as we can so we are being measured about how we approach this so that it will be beneficial to the community. Please let @kfcampbell or me (@nickfloyd) know if y'all have any questions or want more information. |
Does this mean you're designing the client side SDK for GitHub? We (Microsoft Open Source Program Office and the .NET teams) are heavy users of the GitHub API (Ocotkit.NET, including the GraphQL version). If you're in the middle of redesigning it we'd love to give you some feedback. Happy to do a call or if you prefer we can give you a write up, whatever is easier. |
We'd love some feedback and a discussion! If you (or anyone else on this thread) get a chance please jot down your thoughts on this discussion thread and I'll reach out to you via teams to see if we can hop on a call depending on your availability. Thank you for making time for this; we need feedback, ideas, and insight if we're going to deliver something that the community will actually need/want. |
There was a relevant blog post published yesterday: https://devblogs.microsoft.com/dotnet/creating-aot-compatible-libraries/ |
Yes, @eerhardt and I were talking about this. I think the decision has been made with this project to source generate the clients instead of taking the time/effort to trim here. But I could be wrong. |
That is correct. Instead of trying to invest heavily in converting Octokit.net we are beginning the designs and ideation on a generated SDK (using Kiota) that will provide features like trimming out of the box. We're not quite ready to EoL Octokit.net or anything just yet but we feel with y'alls help and input we're going to be able to create something that is not only sustainable but a thriving tool that will help us all build the next generation of tooling. ❤ Please consider getting involved. Y'all are getting a sneak peek of things to come - we'll be dropping a blog post with details and a call to action and more details on how to get involved if you want to - we know we'll need everyone's input to get this moving in the right direction so I am really hoping y'all decide to join us on this new adventure! 🗻 Side note: We are so grateful for each of you! The fact that you care so much about the work you do and the community you're in is super inspiring. Thank you all for what you do for our industry and your users. |
👋 Hey Friends, this issue has been automatically marked as |
What happened?
When you build a .NET executable with trimming enabled, the linker will produce trim warnings about Octokit because it is not trim compatible:
To fix this, Octokit needs to add the following properties to the C# project files:
Versions
6.2.1
Relevant log output
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: