Open mlahp7 opened 5 months ago
PR looks great, does this solve for #134 ?
Thanks, yes it does for the most part, minus the location names that was mentioned in their issue. I don't mind tacking them on this PR.
I'm a bit new to singer and taps in general, is the best course of action to add a new setting for this as I've done with the other two configs or is there another preferred way to add these types of configurations?
PR looks great, does this solve for #134 ?
Thanks, yes it does for the most part, minus the location names that was mentioned in their issue. I don't mind tacking them on this PR.
I'm a bit new to singer and taps in general, is the best course of action to add a new setting for this as I've done with the other two configs or is there another preferred way to add these types of configurations?
If you could update the README portion of Time Entries to include where this PR would bring us to I'd be good with that. For the config's yes you're doing it right, the only thing you could do on top of that is update the configuration readme portion of the docs (if you run poetry run tap-clickup --about --format=md
it'll output markdown for you to copy paste!
PR looks great, does this solve for #134 ?
Thanks, yes it does for the most part, minus the location names that was mentioned in their issue. I don't mind tacking them on this PR. I'm a bit new to singer and taps in general, is the best course of action to add a new setting for this as I've done with the other two configs or is there another preferred way to add these types of configurations?
If you could update the README portion of Time Entries to include where this PR would bring us to I'd be good with that. For the config's yes you're doing it right, the only thing you could do on top of that is update the configuration readme portion of the docs (if you run
poetry run tap-clickup --about --format=md
it'll output markdown for you to copy paste!
Done
Is there something else we need to improve to get this merged?
Is there something else we need to improve to get this merged?
Took another peek and left a review!
Notes: _The time_entry_start_date might be overkill but currently there isn't a query param we can use for this endpoint that allows us to query for entries that have changed. Something like date_updated_gt from the get tasks endpoint would be great here_
_The new default behavior for time entries will be to get the entries of ALL users in your team. If you want only a subset of assignees you can use the time_entry_assignees conf._