Skip to content
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

create method abstraction to allow to be called from a module #43

Closed
wants to merge 1 commit into from
Closed

create method abstraction to allow to be called from a module #43

wants to merge 1 commit into from

Conversation

tgrecojr
Copy link

I created a new method called get_garmin_connect_data in order to more easily allow this code to be called from another python script as a module. The use case here is to be able to script the execution of the downloads in conjunction with other activities (such as parsing and inserting to a database) without having to pass the command in via command line.

The original argument parsing remains intact if called from the command line.

@khrapovs
Copy link
Contributor

Two thumbs up for the idea! I did a similar implementation in my own fork. Basically moved garminbackup.py to garminexport/garminbackup.py and broke it into two functions. Another difference is that the only argument is a single object args with attributes instead of a long list of specific arguments like username, password, etc. Maybe I should also try my luck with a pull request? ;)

@tgrecojr
Copy link
Author

If you could send a PR, that would be awesome! I hope at some point It gets merged. I like your implementation very much — more the “python way”.

@khrapovs
Copy link
Contributor

Here you go: #44

@tgrecojr
Copy link
Author

I am going to close this pull request in favor of #44 by @khrapovs . The idea is the same, but the implementation is better. I would love if you could merge #44 .

@tgrecojr tgrecojr closed this Feb 28, 2020
@petergardfjall
Copy link
Owner

Thank you @tgrecojr ! I've taken a look at #44 and am, generally in favor of the idea, but I think the proposal needs some work in order to become a generally useful client API (refer to the review for detalis). Thanks!

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.

None yet

3 participants