search

ChristopherHaws / gdax-dotnet

DotNet Core GDAX Exchange API
MIT License
7 stars 7 forks source link

Market Period Enum Added and some tests #6

Closed InTheta closed 6 years ago

InTheta commented 6 years ago

Market Period Enum Added and some tests

ChristopherHaws commented 6 years ago

Thanks for the PR. I am working on getting an automated build setup, so I will probably not be able to merge this PR until this weekend. I am also working on making it easier to write unit tests without having to use the sandbox site since those wont be able to run in the automated build. I will probably make a second test project for integration tests that wont run as part of the CI.

InTheta commented 6 years ago

Sounds good.

Need to implement websocket too. I don't really need it right now, everything else I have needed for a project I'm working on.

Only major change is the MarketPeriod Enum addition.

The rest was cleaning up the test files.

All the best

ChristopherHaws commented 6 years ago

I think that it might make more sense to use TimeSpan instead of an enum for the period. Then consumers aren't locked into the values in the enum.

InTheta commented 6 years ago

You will 95% of the time you will use a standard chart period.

You don't need a 7 min chart etc. Standard charting periods are more useful I would say.

Just my opinion.

On 6 Oct 2017, at 18:23, Christopher Haws notifications@github.com wrote:

I think that it might make more sense to use TimeSpan instead of an enum for the period. Then consumers aren't locked into the values in the enum.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.