Skip to content

Conversation

jaredpar
Copy link
Member

@jaredpar jaredpar commented Jan 7, 2022

No description provided.

@jaredpar jaredpar marked this pull request as ready for review January 7, 2022 15:35
@jaredpar jaredpar requested a review from a team as a code owner January 7, 2022 15:35
@jaredpar
Copy link
Member Author

jaredpar commented Jan 7, 2022

@roslyn-compiler PTAL

Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

Done review pass (commit 2)

Comment on lines +119 to +121
/// <summary>
/// <inheritdoc/>
/// </summary>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// <summary>
/// <inheritdoc/>
/// </summary>
/// <inheritdoc/>

I think this is all you need, though @sharwell can correct me if I'm wrong.

@jaredpar
Copy link
Member Author

@dotnet/roslyn-compiler can I get one more review here?

}

var sourceModuleSymbol = this.ContainingModule as SourceModuleSymbol;
return (object)sourceModuleSymbol == null ? null : sourceModuleSymbol.DeclaringCompilation;
Copy link
Contributor

Choose a reason for hiding this comment

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

(object)

Why remove the cast?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah good catch. I was undoing edits in this file (had temp converted it to ?.). Apparently didn't undo far enough.

@jaredpar jaredpar enabled auto-merge (squash) January 11, 2022 20:18
@jaredpar jaredpar merged commit 1b30a3b into dotnet:main Jan 12, 2022
@ghost ghost added this to the Next milestone Jan 12, 2022
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P1 Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants