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.

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

Consider extracting Mock implementations from main library #107

Open
paulomorgado opened this issue Oct 15, 2018 · 10 comments
Open

Consider extracting Mock implementations from main library #107

paulomorgado opened this issue Oct 15, 2018 · 10 comments

Comments

@paulomorgado
Copy link

As I understand it, Mock implementations are to be only used in unit tests. So, they should be on their own assembly/package.

@cwe1ss
Copy link
Member

cwe1ss commented Oct 15, 2018

Do you have a use case for why having them in the same assembly is problematic? These classes are very simple and don't add any dependencies to the main library. Extracting them would complicate using the libraries for developers IMO.

@paulomorgado
Copy link
Author

I don't have any problematic use case. I just don't think it's a good practice to throw everything and the kitchen think in an assembly just because it might be useful in very particular scenarios.

@austinlparker
Copy link
Member

@paulomorgado - I see this is your first issue; Welcome!

The Mock implementation can be useful in scenarios outside of unit testing, IMO - for instance, example/sample programs. I also don't see a really compelling reason to split them out.

@paulomorgado
Copy link
Author

@austinlparker, in other words, not in production scenarios.

@austinlparker
Copy link
Member

Could you describe the production use case where having them in the same assembly is problematic? Alternately, could you make a PR with the changes you're suggesting? I don't think there would be any real problem with multiple assemblies in the NuGet package.

@paulomorgado
Copy link
Author

It's more a case of separation of concerns at this moment than being technically problematic.

And I'm not talking about multiple assemblies in the same package but multiple packages.

@austinlparker
Copy link
Member

As long as the mock tracer remained in this repository and was a project reference so it remained easy to keep up to date with changes to the interface itself, I don't think there'd be much of a problem. That said, if you feel very strongly about this I'd recommend making a PR for it.

@cwe1ss
Copy link
Member

cwe1ss commented Oct 15, 2018

I'd prefer some more opinions from other people before you create a PR (and invest your time). I understand your reasoning from a theoretical software architecture point of view, but I feel quite strongly against it as it mainly adds complexity and doesn't add any real value.

@cwe1ss
Copy link
Member

cwe1ss commented Oct 15, 2018

fyi, we've already briefly discussed this in #55 (comment) back in February.

@carlosalberto
Copy link
Contributor

I personally agree with Christian on this one (I remember opposing the current organization at first). I think the advantage of the simplicity outweights other factors, specially given this is an API assembly, so it is rather small anyway.

My advise: if/when this design becomes a problem, splitting the mock and util layers could be considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants