Skip to content

Conversation

@austindrenski
Copy link
Contributor

Follows #685.

@roji
Copy link
Member

roji commented Dec 2, 2018

Deferring on this until we decide on #685.

@austindrenski austindrenski force-pushed the metadata-ranges branch 2 times, most recently from dee74b9 to d78eff9 Compare December 2, 2018 16:00
@austindrenski austindrenski force-pushed the metadata-ranges branch 2 times, most recently from 111ecdf to 78067af Compare December 6, 2018 21:19
@austindrenski austindrenski requested a review from roji December 6, 2018 21:20
@austindrenski
Copy link
Contributor Author

@roji This is updated and ready for review. It follows #685, so filter on the last commit for review.

@austindrenski
Copy link
Contributor Author

Rebased for #685.

/// </summary>
/// <param name="annotatable">The annotatable to search for the annotation.</param>
/// <param name="annotationName">The annotation name to search for in the annotatable.</param>
/// <exception cref="ArgumentNullException"><paramref name="annotatable"/></exception>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these exception xmldocs have much value (not sure I've actually seen them specified in other projects etc.). I'm especially skeptical of documenting each and every internal method, it seems to bloat the source for very little gain...

But let's move on...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, they show up in the on-hover documentation, which is nice for a quick check on whether or not a null parameter is going to throw.

@austindrenski austindrenski merged commit d644272 into npgsql:dev Dec 6, 2018
@austindrenski austindrenski deleted the metadata-ranges branch December 6, 2018 22:07
@roji roji added cleanup and removed refactor labels May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants