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

Fix an issue when using classes with similar APIs. #40

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

Conversation

matthid
Copy link

@matthid matthid commented Apr 5, 2022

When you use Exposed on similar APIs you will run into issues as the DLR will cache requests and fail (or even worse return wrong results)

@skolima
Copy link
Owner

skolima commented Apr 5, 2022

Hi @matthid , this looks like a really well prepared PR! Thank you very much. I'll do my best to check it in detail soon.

@skolima
Copy link
Owner

skolima commented Apr 22, 2022

Hey. I gave this a go, unfortunately it doesn't include the code path for versions without nullable annotation support. I could bump the library to be only netstandard2.0 and net6.0 (like e.g. NodaTime is now) and see what approach Skeet takes to make this work (I though nullable annotations don't work at all on netstandard). I'll be back after bumping the targets...

@matthid
Copy link
Author

matthid commented Apr 22, 2022

Yes I was a bit confused what was going on there. I thought Nullable is a compiler feature, and you can increase the compiler version to latest https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/configure-language-version and just enable nullable (even for old net framework). This way you should be able to get rid of the compiler directive. I'm not sure if this public-api check you have in place supports that though.

@skolima
Copy link
Owner

skolima commented May 18, 2022

I've dropped the older targets and the path without nullable annotations from the main branch, but I'm now having issues merging this back into your branch. I keep getting NPEs when testing static members. Could you have a got with updating your branch from my main please?

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

Successfully merging this pull request may close these issues.

2 participants