Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Conversation

@victorhurdugaci
Copy link
Contributor

@Eilon, in response to your comment here:

IVT is to be used only for the unit test assembly of this assembly. Make the type(s) in question public and move to a .Internal namespace.

Couldn't move to an internal namespace because it is just a method. Also, moving it to an extension method would be strange because this method needs access to private fields.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a doc comment to this, perhaps warning that it's for testing scenarios only?

Copy link
Contributor

Choose a reason for hiding this comment

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

(And add a doc comment to the other Add() method while you're at it? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Btw, the Add overload with 2 arguments is just on this implementation. Most users will use configuration through IConfiguration which doesn't expose the overload

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see, that's good to know. Good to document anyway 😄

@victorhurdugaci
Copy link
Contributor Author

Documented

Copy link
Contributor

Choose a reason for hiding this comment

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

And all sentences with periods (summary, params, returns, etc.).

@victorhurdugaci
Copy link
Contributor Author

Updated with magic doc ref

@Eilon
Copy link
Contributor

Eilon commented Apr 29, 2015

:shipit: !

@victorhurdugaci victorhurdugaci merged commit 10ed680 into dev Apr 29, 2015
@victorhurdugaci victorhurdugaci deleted the victorhu/arrays-pr branch April 29, 2015 19:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants