-
Notifications
You must be signed in to change notification settings - Fork 18
Add mirror env variable when set by the user #423
Conversation
@@ -17,6 +17,7 @@ export function detectLaunchConfigurationChanges( | |||
"serverProperties", | |||
"javaHome", | |||
"customRepositories", | |||
"coursierMirror", |
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.
Can we extract these keys as constants and use them in vscode?
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.
In a couple of places yes, but unfortunately not in package.json
which would be most useful
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.
That's true, but at least we can reduce 3 possible sources of errors (config options names) to just 2
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.
Added!
src/interfaces/UserConfiguration.ts
Outdated
* | ||
* - https://scalameta.org/metals/docs/integrations/new-editor#metals-user-configuration | ||
*/ | ||
export const UserConfiguration = { |
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 you can use enum, with it exporting type shouldn't be necessary
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.
Changed!
This can be found https://github.com/coursier/coursier/blob/master/modules/paths/src/main/java/coursier/paths/Mirror.java#L26
Tested it out and seems to work (URLs go replaced by my string)
Will create VS Code PR soon.