-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Makes GitLens XDG-compatible #3904
base: main
Are you sure you want to change the base?
Makes GitLens XDG-compatible #3904
Conversation
} | ||
|
||
// export as a singleton | ||
// eslint-disable-next-line import-x/no-default-export | ||
export default new SharedGKDataFolderMapper(); |
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.
Don't use a default export
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.
ok
5f1a57e
to
75caad1
Compare
75caad1
to
3d08906
Compare
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.
Works for me, just a few minor comments and suggestions.
} catch { | ||
// File does not exist, so we can safely create it | ||
break; | ||
// Path does not exist, so we can safely use xdg paths it |
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.
// Path does not exist, so we can safely use xdg paths it | |
// Path does not exist, so we can safely use xdg paths |
break; | ||
// Path does not exist, so we can safely use xdg paths it | ||
const platform = getPlatform(); | ||
const folderName = 'gk'; |
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.
Should this be GitKraken
? Would follow a better convention perhaps with the rest of folders in places like AppData/Local
. Thoughts @eamodio ?
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.
I think we should follow the exact convention of the current folder name/structure -- just in a different location
class SharedGKDataFolderMapper { | ||
private _initPromise: Promise<void> | undefined; | ||
constructor( | ||
// do soft migration, use new folders only for new users (without existing folders) |
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.
We may want to inform users who have a .gk
folder, and want to use the new location, like many of the users in the original issue, that they should delete the former. Otherwise GitLens will continue using it.
In fact, they may want to take the contents out of that folder, delete it, and then move it into the new XDG-friendly folder so they don't lose the data.
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.
Yeah, probably in the help center and on the issue, but I wouldn't say in the app
Description
All new installs files will be placed into platform specific folder
Existing installs are used $HOME.gk directory. It can be changed manually by deleting .gk directory or moving to corresponding directory
https://github.com/sindresorhus/env-paths/tree/main could be used instead, it implements the logic
Checklist
Fixes $XXX -
orCloses #XXX -
prefix to auto-close the issue that your PR addresses