-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Entry point for completed/get_all #30
Conversation
@@ -365,6 +365,15 @@ export const Todoist = (token: string, userOptions = defaultOptions) => { | |||
get: (options: any) => request({ url: `${options.endpoint}/activity/get`, query: options }), | |||
} | |||
|
|||
const completedItems = { | |||
get: async (opts: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why get()
rather than getAll()
? The later has the advantage of conserving a 1-1 mapping with the API's method names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem for completedItems
vs completed
.
Looks good but the |
Oh, my bad. I committed dist because it was easier for me to use the fork for my personal use, but it makes the PR quite messy. In additions, you've made a few legit comments about the code itself. The update is small. You can drop this PR and simply pick the parts you want and adapt them yourself if you're comfortable with this approach. |
And by the way, thank you for making this project! |
Thanks but I'll close the PR if you don't wish to complete it. You or anyone else is welcome to re-open at anytime. |
A new entry point
completedItems
for https://developer.todoist.com/sync/v8/#get-all-completed-items