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

Add a new option --logpath #74

Merged
merged 6 commits into from
Jun 7, 2022
Merged

Add a new option --logpath #74

merged 6 commits into from
Jun 7, 2022

Conversation

bxsx
Copy link
Contributor

@bxsx bxsx commented May 31, 2022

Summary

This new option allows declaring a target directory for log files.
The default value is pulled from the --directory option.

IMPORTANT

This is a breaking change.

Old default behavior

gcexpert.log is stored in the current application directory

New default behavior

gcepxport.log is stored in the target directory of --directory, so the default is ./YYYY-MM-DD_garmin_connect_export

Motivation

By default, the application stores the exported activities in daily directories such as YYYY-MM-DD_garmin_connect_export.
By storing daily log files in the same directory as the activities, the user would have the full package of all files created by the app.

It is also worth noting that many operating systems have dedicated log folders, e.g. /var/log/ etc. This option allows for storing logs in such locations.

Background

Initial commit 7ab7125 by @cristian5th was cherry-picked from PR #66 where you can also find more discussion about this option.

cristian5th and others added 4 commits May 31, 2022 23:23
This patch is copied from the export_data_file()
This option allows to set a directory destionation for logfiles.
If unset a value from --directory is used.
@bxsx
Copy link
Contributor Author

bxsx commented May 31, 2022

I added general cleanups for the parser in 57e830f. It can be reverted if not desired.

@pe-st
Copy link
Owner

pe-st commented Jun 2, 2022

Thanks, I will merge it soon

@pe-st pe-st merged commit 2cde59e into pe-st:master Jun 7, 2022
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.

3 participants