Skip to content

Conversation

@maxkoshevoi
Copy link
Contributor

Related to #43605

Questions:

  • Not sure if failureMessage should be nullable, but there's never a null-check for it
  • In OptionsFactory.Create name parameter looks like it should be nullable, but OptionsValidationException will throw if name is null

Other questions:

  • dotnet msbuild /t:GenerateReferenceAssemblySource doesn't update nullable annotations. How can I update ref files automatically? (there were a lot of ?s in this PR)

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-Extensions-Options new-api-needs-documentation labels Jan 13, 2022
@ghost
Copy link

ghost commented Jan 13, 2022

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Jan 13, 2022

Tagging subscribers to this area: @dotnet/area-extensions-options
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #43605

Questions:

  • Not sure if failureMessage should be nullable, but there's never a null-check for it
  • In OptionsFactory.Create name parameter looks like it should be nullable, but OptionsValidationException will throw if name is null

Other questions:

  • dotnet msbuild /t:GenerateReferenceAssemblySource doesn't update nullable annotations. How can I update ref files automatically? (there were a lot of ?s in this PR)
Author: maxkoshevoi
Assignees: -
Labels:

new-api-needs-documentation, area-Extensions-Options, community-contribution

Milestone: -

@eerhardt
Copy link
Member

dotnet msbuild /t:GenerateReferenceAssemblySource doesn't update nullable annotations

Hmmm, I thought #61760 fixed this. @safern?

@eerhardt eerhardt self-assigned this Jan 21, 2022
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for all your hard work here @maxkoshevoi!

LGTM

@eerhardt eerhardt merged commit bf0492f into dotnet:main Feb 11, 2022
@maxkoshevoi maxkoshevoi deleted the mk/43605-options branch February 11, 2022 16:45
@ghost ghost locked as resolved and limited conversation to collaborators Mar 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Extensions-Options community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants