search

aarondcoleman / Fitbit.NET

Fitbit .NET API Client Library
MIT License
193 stars 138 forks source link

Remove reliance on FitbitAppCredentials from FitbitClient #247

Open WestDiscGolf opened 5 years ago

WestDiscGolf commented 5 years ago

On tidying up the repository I noticed a number of issues I want to resolve. These will most likely be design and/or architectural based issues which are stopping or hindering us moving forward.

First up - Remove reliance on FitbitAppCredentials from FitbitClient

We currently pass in the Fitbit app credentials every time we construct an instance of FitbitClient however these do not change, the FitbitClient does not use them and they are only used in the DefaultTokenManager. This is not good.

Please see the details of the pull request here - https://github.com/WestDiscGolf/Fitbit.NET/pull/2

I will submit a proper PR once the other PR #246 has been approved and merged.

aarondcoleman commented 5 years ago

Hi Adam! We can’t accept any structural or large delta code changes now. Our code that wraps this library would be way too much to refactor with little functional value.

In our case what we do is have a factory pattern class that generates the FitbitClient where needed (and per each needed Fitbit account). So there’s really not a ton of excess here since each time we switch profiles to pull data, we just pass our internal id to the factory and it news-up an instance of FitbitClient.

Cheers, —Aaron

On Sat, Jan 5, 2019 at 3:38 PM Adam Storr notifications@github.com wrote:

On tidying up the repository I noticed a number of issues I want to resolve. These will most likely be design and/or architectural based issues which are stopping or hindering us moving forward.

First up - Remove reliance on FitbitAppCredentials from FitbitClient

We currently pass in the Fitbit app credentials every time we construct an instance of FitbitClient however these do not change, the FitbitClient does not use them and they are only used in the DefaultTokenManager. This is not good.

Please see the details of the pull request here - WestDiscGolf#2 https://github.com/WestDiscGolf/Fitbit.NET/pull/2

I will submit a proper PR once the other PR #246 https://github.com/aarondcoleman/Fitbit.NET/pull/246 has been approved and merged.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/aarondcoleman/Fitbit.NET/issues/247, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ1Cb0Vv1TsWHjfRWycIzDmaCAOuWKGks5vATeHgaJpZM4ZyKSD .

-- Founder & CEO Fitabase by Small Steps Labs LLC @aaronc