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

[RFC] Add a .NET 8.0 target to OpenMcdf.Extensions #123

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Numpsy
Copy link
Contributor

@Numpsy Numpsy commented Mar 7, 2024

I see from the code history that there was once a change to .NET 6.0, that was then reverted to .NET Standard 2.0.
However, I think it would be useful to add an additional .NET 6.0 target on top of the existing ones (rather than removing them).

The main immediate arguments are:

  1. A .NET 6,0 build doesn't need to use the System.Text.Encoding.CodePages NuGet package, as System.Text.Encoding.CodePages is built in to it.
  2. In order to enable new features like IsTrimmable and get analysis warnings about any issues. (I've been trying to use it in a self contained, trimmed build recently, and more early analysis is always useful there)

But also it should be useful for #57 to make span based functions in System.IO.Stream and related available, as there are places that should be able to benefit from things like spans, stackalloced buffers etc (e.g. writing null terminators and padding with spans, instead of writing bytes in loops)

This is just doing the Extensions lib now due to the System.Text.Encoding.CodePages dependency, but the same question exists for the core library.

Thoughts?

@ironfede
Copy link
Owner

ironfede commented Mar 7, 2024

You're right in your analysis but compatibility was (and I think that still is) one of the major goals for OpenMcdf.
Now, this doesn't mean that we can't have platform support upgrade but if you introduce support for net 6 in this branch, all things you're mentioning should be included and developed with conditional version-moniker in the code, I suppose.
Do you think that this could be supported without risking a lot of regression errors and potential pitfalls in development?
In my opinion, pheraps, a new major release targeting .net 6 would be more mantaineable. This would also possibly imply freezing new developments and evolutions on 2.x branch. Do you think it could be a feasible solution?

@Numpsy
Copy link
Contributor Author

Numpsy commented Mar 7, 2024

Do you think that this could be supported without risking a lot of regression errors

I think the main thing would be that if the library targeted .NET 4.x / 6.0 / Standard 2.0 and the unit tests target 4.x/6.0 then the Standard build wouldn't get covered (you could add another earlier version to the tests easily enough, with the caveat that .NET Core 3.1 and .NET 5.0 are both out of support)

Another thing is that you can potentially use a subset of the Span features in .NET 4.x via the System.Memory NuGet package, but I think not all the extra functions that have been added to Streams etc (as an example, the metadata extractor library has recently been having some work to use spans done, whilst still supporting .NET 4.6.2)

@jeremy-visionaid
Copy link
Contributor

@Numpsy Thanks for looking at this!

@ironfede I'd love to use OpenMcdf in one of our projects. I think we're going to want to add .net 6, spans etc. to it, but also some other likely breaking changes to improve the API and increase performance, reduce memory footrpint, possibly trimmable support, etc. My company and others I work with are happy to make contributions to that end. We're going to need to ship in a few months though - Is there anything that would be useful to assist in moving towards a v3 release?

@Numpsy
Copy link
Contributor Author

Numpsy commented Sep 2, 2024

I started a bit of span stuff in #125 though just the suggestions in #57 and to get comments on bumping the minimum supported .NET version vs. conditionally compiling the span related functions for different .NET versions.

possibly trimmable support

fwiw I'm already using it in trimmed builds (the only warning I've seen in that area was fixed in #126)

@Numpsy Numpsy changed the title [RFC] Add a .NET 6.0 target to OpenMcdf.Extensions [RFC] Add a .NET 8.0 target to OpenMcdf.Extensions Sep 17, 2024
@Numpsy Numpsy mentioned this pull request Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants