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.

Sign up for GitHub

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

Jump to bottom

Fix SyntaxFactory.XmlSeeAlsoElement href overloads #75052

Merged
merged 1 commit into from
Sep 17, 2024
Merged

Fix SyntaxFactory.XmlSeeAlsoElement href overloads #75052

merged 1 commit into from
Sep 17, 2024

Conversation

bricelam
Copy link
Contributor

Fixes #54201

@bricelam bricelam requested a review from a team as a code owner September 11, 2024 17:48
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 11, 2024
@dotnet-policy-service dotnet-policy-service bot added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Sep 11, 2024
bricelam
bricelam commented Sep 11, 2024
View reviewed changes
src/Compilers/VisualBasic/Portable/Syntax/SyntaxFactory.vb
XmlName(Nothing, XmlTextLiteralToken(DocumentationCommentXmlNames.CrefAttributeName, DocumentationCommentXmlNames.CrefAttributeName)),
XmlName(Nothing, XmlNameToken(DocumentationCommentXmlNames.HrefAttributeName, SyntaxKind.XmlName)).WithLeadingTrivia(ElasticSpace),
Copy link
Contributor Author

@bricelam bricelam Sep 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Note, this method would always throw ArgumentException before this change. (Because, per the grammar, XmlTextLiteralToken is an invalid token type for XmlName)

@333fred
Copy link
Member

333fred commented Sep 11, 2024

@dotnet/roslyn-compiler for a second review.

AlekseyTs
AlekseyTs approved these changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 1)

@333fred 333fred merged commit 6a59e0f into dotnet:main Sep 17, 2024
24 checks passed
@333fred
Copy link
Member

333fred commented Sep 17, 2024

Thanks @bricelam!

@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Sep 17, 2024
@bricelam bricelam deleted the href branch September 18, 2024 13:57
This was referenced Sep 20, 2024
Closed
Closed
@akhera99 akhera99 modified the milestones: Next, 17.12 P3 Sep 26, 2024
@dotnet-bot dotnet-bot mentioned this pull request Oct 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@333fred 333fred 333fred approved these changes

@AlekseyTs AlekseyTs AlekseyTs approved these changes

Assignees
No one assigned
Labels
Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Milestone
17.12 P3
Development

Successfully merging this pull request may close these issues.

XmlSeeAlsoElement uses 'cref' instead of 'href'
4 participants
@bricelam @333fred @AlekseyTs @akhera99