search

davezych / shience

A .NET port(ish) of Github's Scientist library. (https://github.com/github/scientist)
MIT License
9 stars 1 forks source link

Code Cleanup and Fluent Interface #13

Closed MovGP0 closed 8 years ago

davezych commented 8 years ago

I noted a few build errors that need fixing, otherwise looks good.

davezych commented 8 years ago

Oh, also, the Readme needs to be updated to reflect the fluent interface.

davezych commented 8 years ago

One more thought: should the control and candidate be moved out to their own methods? i.e. instead of:

science.Test("test-name", control: () => { /*whatever*/ }, candidate: () => { /*Whatever */ })

something like:

science.Test("test-name")
            .Control(() => { /*whatever*/ })
            .Candidate(() => { /*Whatever */ }
            .Execute()

Methods could be named Use/Try as well to stay consistent with the wording that Scientist uses.

MovGP0 commented 8 years ago

sorry for the build errors. could not fix them because the reference in the test project was not working properly.

MovGP0 commented 8 years ago

the build errors from #14 are fixed and #16 is solved too

davezych commented 8 years ago

When I pull this down and restore packages the test lib is not able to find the Shience package. If I move the dependency into the dnxcore50 framework I'm able to restore. Not sure why that matters, seeing as we're only targeting dnxcore50 anyway. You didn't run into that?

Everything else looks good, once we get this figured out I'll merge it.

MovGP0 commented 8 years ago

strange. on my machine both versions are working. not sure what the difference is.

davezych commented 8 years ago

I changed the casing from Shience to shience and it restored. I'm going to chalk it up to a bug in tooling.

This all looks good, merging... :shipit: