-
Notifications
You must be signed in to change notification settings - Fork 82
Solution/Project update #156
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
Solution/Project update #156
Conversation
Thank you very much @jeremy-visionaid for your PRs. I'm reviewing all your changes. Do you have any idea about why this happens? Anyhow, It's ok to me using StyleCop but , if You agree, not in 2.x branch. I would avoid introducing new dependencies in a long existing release/branch of OpenMcdf to avoid a dependency graph change for current user base. I would go to a total feature freeze in 2.x branch for a final 2.4 release supporting old .net framework. 3.x should targeting .net 8 exclusively imho. Please let me know If this sounds good to you and to evrerybody with interests in OpenMcdf reading this comment. |
@ironfede That's curious about the warning - I don't get it on mine. Maybe try a close, WRT. to introducing additional dependencies. the PackageReference has It should be possible to drop the IncludeAssets element too: It's totally up to you whether you want to use StyleCop in v2, v3, or not at all. I think just the important thing is to get the style changes that it suggests in (whether that's enforced or not). Otherwise it will probably just be too much work to get bug fixes in that might turn up while working on v3. Personally, I'm happy with things being dotnet 8 only - I have no requirement for anything else. I can't speak for others, but I'd be happy also supporting .NET Standard 2.0/2.1 if you wanted to do so. There would be a bunch of code that could be dropped with dotnet 8 only, and it would be easier to provide async methods. From a migration perspective, the only problematic thing I notice is that I haven't found a comparable Hex Editor control, there's only a viewer... Did you want me to just drop the StyleCop config from the end of the PR for now if that's the only thing blocking the rest of it? |
Ok with style changes but StyleCop only in v3.
Sorry, I've been too tranchant in the comment...
Yes please Thank you @jeremy-visionaid |
If there is still a .NET Standard 2.0 build of the lib (rather than just making it .NET 8) then StructuredStorageExplorer could stay on ,NET 4.8, at least to begin with? |
Yeah, I think if netstandard2.0 is being kept then the core libraries could just be made netstandard2.0 only. IIRC, net48 is the only .NET Framework version that's currently supported (and is also covered by netstandard2.0), so net40 should just be dropped. StructuredStorageExplorer could then stay net48 and all the tests etc. could be net8.0. I'd then perceive adding net8.0 to the core libraries as optional extra work - both paths would need to be supported but we'd be unable to take advantage of a lot of its new APIs etc. in the common code. I'm happy either way, but netstandard2.0 only might just be easier. |
8f6965a
to
e71ea0b
Compare
@ironfede I've dropped StyleCop config from the PR as requested |
Personally, I'd like an official build for .NET x.0 that can remove the dependency on the System.Text.Encoding.CodePages NuGet package - e.g. #123 - which isn't itself much extra work (and then you also get better support for trimming/AOT analysis). |
This does a few things:
I added a placeholder .globalconfig from one of my own projects for the analyzer configuration, but you should definitely check it out locally first to see how you'd like it handled (if at all, but it gives about 3.5k warnings up from 200 currently):
https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files
Basically there are 3 options:
So, I'd need to update it based on your preferences - I would obviously try and be as sympathetic as possible to the existing style of the project. The main intent here is that by specifying the style and enforcing it, it's more likely that we'd be able to cherry-pick and/or merge fixes cleanly between 2.x and 3.x.
Just let me know if you'd like me to re-work or drop any of it 👍