search

opentracing / opentracing-csharp

OpenTracing API for C# (.NET). 🛑 This library is DEPRECATED! https://github.com/opentracing/specification/issues/163
http://opentracing.io
Apache License 2.0
517 stars 73 forks source link

Consider extracting Mock implementations from main library #107

Open paulomorgado opened 5 years ago

paulomorgado commented 5 years ago

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

cwe1ss commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

austinlparker commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

fyi, we've already briefly discussed this in https://github.com/opentracing/opentracing-csharp/issues/55#issuecomment-363588253 back in February.

carlosalberto commented 5 years ago

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.