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

Default to ~/.config/gmailctl for newly created config dirs #393

Closed
wants to merge 2 commits into from

Conversation

lgarron
Copy link

@lgarron lgarron commented Mar 21, 2024

This is a minimal fix for #144

cmd/gmailctl/cmd/root_cmd.go Outdated Show resolved Hide resolved
Copy link
Owner

@mbrt mbrt left a comment

Choose a reason for hiding this comment

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

Thanks for the contributions!

There are also a bunch of references to the old directory in the main README.md. Could you update them?

I think the new behavior should also be explained in the gmailctl init --help message. Something along the lines that the default config directory location moved from one location to the other.

Sorry for the back and forth, I missed these points in the earlier review. Thanks!

@mbrt
Copy link
Owner

mbrt commented Mar 23, 2024

I thought about this a bit more and the problem with this PR is that it doesn't solve the problem for all platforms. I think the original issue is more about OSX and Windows, where polluting the home directory with dotfiles is not common.

I'm afraid this just introduces one more variant without really tackling the underlying issue.

@lgarron
Copy link
Author

lgarron commented Mar 23, 2024

I thought about this a bit more and the problem with this PR is that it doesn't solve the problem for all platforms. I think the original issue is more about OSX and Windows, where polluting the home directory with dotfiles is not common.

I'm afraid this just introduces one more variant without really tackling the underlying issue.

I mean, that's fine if you'd prefer to match OS conventions more closely. How would you feel about using https://github.com/adrg/xdg to compute the config path?

@mbrt
Copy link
Owner

mbrt commented Mar 24, 2024

Using adrg/xdg seems better to me. We would make the behavior more complicated, but at the same time behave in a more standard way. I also took a look at the library and it seems well maintained and low dependency.

This should be explained decently in the docs, but I would agree with that change.

Copy link
Contributor

This pull request is stale because it has been open for 30 days without activity.
This will be closed in 7 days, unless you add the 'lifecycle/keep-alive' label or comment.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity, has become stale and will be auto-closed. label Apr 24, 2024
@github-actions github-actions bot closed this May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity, has become stale and will be auto-closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants