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

Add MemberNotNullWhen annotation for Content / HasContent pair #1779

Conversation

hansmbakker
Copy link
Contributor

What kind of change does this PR introduce?

It adds the MemberNotNullWhen annotation to the Content / HasContent pair in ApiException.

What is the current behavior?

VS does not recognize Content being non-null when HasContent is true.

What is the new behavior?

VS should recognize Content being non-null when HasContent is true.

What might this PR break?

Please check if the PR fulfills these requirements

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

Other information:

@hansmbakker
Copy link
Contributor Author

This is a continuation of #1302

@hansmbakker hansmbakker marked this pull request as ready for review August 5, 2024 13:39
@ChrisPulman ChrisPulman enabled auto-merge (squash) August 9, 2024 07:38
@hansmbakker
Copy link
Contributor Author

@ChrisPulman I don't see how the error in the tests is linked to my change - can you please advise what to do?

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:01.71] Skipping: Refit.Tests (could not load dependent assembly 'Refit, Version=7.1.0'): Could not load file or assembly 'Refit, Version=7.1.0.0, Culture=neutral, PublicKeyToken=2f9b1262776509f5' or one of its dependencies. Strong name signature could not be verified.  The assembly may have been tampered with, or it was delay signed but not fully signed with the correct private key. (Exception from HRESULT: 0x80131045)

@ChrisPulman
Copy link
Member

@ChrisPulman I don't see how the error in the tests is linked to my change - can you please advise what to do?

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:01.71] Skipping: Refit.Tests (could not load dependent assembly 'Refit, Version=7.1.0'): Could not load file or assembly 'Refit, Version=7.1.0.0, Culture=neutral, PublicKeyToken=2f9b1262776509f5' or one of its dependencies. Strong name signature could not be verified.  The assembly may have been tampered with, or it was delay signed but not fully signed with the correct private key. (Exception from HRESULT: 0x80131045)

I will take a look later, thank you.

Bump Version due to API change
@ChrisPulman ChrisPulman enabled auto-merge (squash) August 17, 2024 02:27
@ChrisPulman ChrisPulman merged commit 1c56e32 into reactiveui:main Aug 17, 2024
1 check passed
@ChrisPulman
Copy link
Member

@ChrisPulman I don't see how the error in the tests is linked to my change - can you please advise what to do?

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:01.71] Skipping: Refit.Tests (could not load dependent assembly 'Refit, Version=7.1.0'): Could not load file or assembly 'Refit, Version=7.1.0.0, Culture=neutral, PublicKeyToken=2f9b1262776509f5' or one of its dependencies. Strong name signature could not be verified.  The assembly may have been tampered with, or it was delay signed but not fully signed with the correct private key. (Exception from HRESULT: 0x80131045)

I will take a look later, thank you.

You had not run the tests, this would have led to a failure of the API test due to a change in the API. I added these tests in to ensure that the version is correctly managed.
I got heavily criticised for making a breaking change without bumping the version, now I can easily check if any of the API files have changed between the last release and this release and depending upon the changes made set the correct version. Hopefully this helps you to understand both why it was failing and the reason for me adding the API tests.

@hansmbakker
Copy link
Contributor Author

I understand, thank you for helping me!

It was just that the build output didn't look like a test failure to me but more like a compilation failure (it couldn't find the DLL).

@hansmbakker hansmbakker deleted the feature/nullability-apiexception-content-hascontent branch August 20, 2024 09:19
Copy link

github-actions bot commented Sep 4, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants