Skip to content

Conversation

@abrammer
Copy link

Takes care of issue #82

fitbit/api.py Outdated
* https://wiki.fitbit.com/display/API/API-Get-Body-Fat
* https://wiki.fitbit.com/display/API/API-Update-Fat-Goal
* https://dev.fitbit.com/docs/body/#body-fat
* https://dev.fitbit.com/docs/body/#log-body-fat
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is for body fat goals, the right URLs are:

https://dev.fitbit.com/docs/body/#get-body-goals (with the fat goal-type)
https://dev.fitbit.com/docs/body/#update-body-fat-goal

fitbit/api.py Outdated
* https://wiki.fitbit.com/display/API/API-Get-Body-Weight-Goal
* https://wiki.fitbit.com/display/API/API-Update-Weight-Goal
https://dev.fitbit.com/docs/body/#goals
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add the update URL here:
https://dev.fitbit.com/docs/body/#update-weight-goal

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I think it makes sense to change the first url to this:
https://dev.fitbit.com/docs/body/#get-body-goals (with the weight goal-type)

fitbit/api.py Outdated
https://wiki.fitbit.com/display/API/API-Get-Activity-Daily-Goals
https://wiki.fitbit.com/display/API/API-Update-Activity-Daily-Goals
https://dev.fitbit.com/docs/activity/#activity-goals
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either change "APIs" to "API" on the line above or specify both the get and update URLs. Can you also modify the line above to mention that this is for the daily version of the API? Something like "Implements the following APIs (where period is daily)
https://dev.fitbit.com/docs/activity/#get-activity-goals
https://dev.fitbit.com/docs/activity/#update-activity-goals

fitbit/api.py Outdated
https://wiki.fitbit.com/display/API/API-Get-Activity-Weekly-Goals
https://wiki.fitbit.com/display/API/API-Update-Activity-Weekly-Goals
https://dev.fitbit.com/docs/activity/#activity-goals
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either change "APIs" to "API" on the line above or specify both the get and update URLs. Can you also modify the line above to mention that this is for the weekly version of the API? Something like "Implements the following APIs (where period is weekly)
https://dev.fitbit.com/docs/activity/#get-activity-goals
https://dev.fitbit.com/docs/activity/#update-activity-goals

fitbit/api.py Outdated
https://wiki.fitbit.com/display/API/API-Get-Food-Goals
https://wiki.fitbit.com/display/API/API-Update-Food-Goals
https://dev.fitbit.com/docs/food-logging/#get-food-goals
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the extra indent please. And add the update URL
https://dev.fitbit.com/docs/food-logging/#update-food-goal

https://wiki.fitbit.com/display/API/API-Get-Water-Goal
https://wiki.fitbit.com/display/API/API-Update-Water-Goal
https://dev.fitbit.com/docs/food-logging/#get-water-goal
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and a 1d period.
https://wiki.fitbit.com/display/API/API-Get-Time-Series
https://dev.fitbit.com/docs/activity/#activity-time-series
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should include all the time series URLs here, and update "url" in the comment above to be "urls"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure what you mean here? The #get-activity-time-series url anchors to literally the next line on the fitbit site. Seems redundant to include to urls for effectively the same link.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abrammer This method doesn't just support activity time series types, it also supports body, food, etc.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, added I think all of them here and covered the other edits in the latest commit c583c7b

send an email to api@fitbit.com and request to have access to the Partner API
(see https://wiki.fitbit.com/display/API/Fitbit+Partner+API). For details on the resources available, see:
fill out the Private Support form here (see https://dev.fitbit.com/docs/help/).
For details on the resources available and more information on how to get access, see:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great 👍

@abrammer
Copy link
Author

abrammer commented Dec 27, 2016

@brad Is this waiting on me for anything? Commit c583c7b should have covered all the requested changes even though a couple still show above.

@brad
Copy link
Member

brad commented Dec 27, 2016

@abrammer Yes, thank you for your contribution!

@brad brad merged commit 3053f47 into orcasgit:master Dec 27, 2016
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 this pull request may close these issues.

2 participants