Skip to content

Conversation

@bollhals
Copy link
Contributor

@bollhals bollhals commented Jul 3, 2024

Proposed Changes

Fixes #1596
Fixes #1622

Enables Nullable Reference Types in the client assembly

I realize a bit late that you intended to do this in 8.0 ... unfortunately I already was 80% through when I did see it 💯

You'll have to decide whether it's worth including it or push it after the snap for 7.0

Types of Changes

  • Bug fix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause an observable behavior change in existing systems)
  • Documentation improvements (corrections, new content, etc)
  • Cosmetic change (whitespace, formatting, etc)

Checklist

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

Further Comments

How does it work with the PublicAPI thing, does this need adjustment as well?

@bollhals
Copy link
Contributor Author

bollhals commented Jul 4, 2024

@lukebakken Any idea what I broke with the public api analyzer that it reports internals on the netstandard2.0 target?

@lukebakken
Copy link
Collaborator

@bollhals somehow this check started to work:

dotnet_diagnostic.RS0051.severity = error

I disabled it (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/Microsoft.CodeAnalysis.PublicApiAnalyzers.md#rs0051-add-internal-types-and-members-to-the-declared-api)

@lukebakken lukebakken marked this pull request as ready for review July 8, 2024 15:22
@lukebakken lukebakken added this to the 7.0.0 milestone Jul 8, 2024
@lukebakken lukebakken self-assigned this Jul 8, 2024
@lukebakken lukebakken requested review from lechu445 and lukebakken July 8, 2024 17:05
@lukebakken lukebakken requested a review from lechu445 July 9, 2024 00:20
@lukebakken
Copy link
Collaborator

@bollhals @lechu445 - anything else? I plan to merge this by 0900PDT today.

@lechu445
Copy link
Contributor

lechu445 commented Jul 9, 2024

@lukebakken nothing more from my side. LGTM

@bollhals
Copy link
Contributor Author

bollhals commented Jul 9, 2024

@bollhals @lechu445 - anything else? I plan to merge this by 0900PDT today.

With your recent commit, not from my side.
Hope it is all annotated corectly 🤞

@lukebakken
Copy link
Collaborator

@bollhals @lechu445 @paulomorgado thank you so much for your insights and contributions here 🥇

@lukebakken lukebakken merged commit 0c8492a into rabbitmq:main Jul 9, 2024
@bollhals bollhals deleted the NRT branch July 9, 2024 21:04
@ngbrown

This comment was marked as resolved.

@lukebakken
Copy link
Collaborator

@ngbrown
Copy link
Contributor

ngbrown commented Sep 27, 2024

@lukebakken yes, you are correct. I had thought I had copied out the most recent version, but evidently not. Sorry about bothering you.

@lukebakken
Copy link
Collaborator

@ngbrown no worries at all! If you're testing out the v7 RC candidates, we would appreciate any feedback you have. Thanks!

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.

NullReferenceException when setting null Uri into ConnectionFactory Nullable Reference Types

5 participants