Skip to content

Conversation

@kenzieschmoll
Copy link
Member

Work towards #6324

final localDartSdkLocation = Platform.environment['LOCAL_DART_SDK'];
if (localDartSdkLocation == null) {
throw Exception('LOCAL_DART_SDK environment variable not set. Please add '
'the following to your \'.bash_profile\' or \'.bash_rc\' file:\n'
Copy link
Member

Choose a reason for hiding this comment

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

nit: .bashrc

Copy link
Member

@elliette elliette left a comment

Choose a reason for hiding this comment

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

Approved with small nit

Comment on lines +83 to +86
throw Exception('LOCAL_DART_SDK environment variable not set. Please add '
'the following to your \'.bash_profile\' or \'.bashrc\' file:\n'
'export LOCAL_DART_SDK=<absolute/path/to/my/dart/sdk>');
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@DanTup how can we tweak this exception to be useful for windows? We can do

final  exceptionMessage =  Platform.isWindows ? ... : ...;
throw Exception(exceptionMessage);

Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably just say something like

Please set the LOCAL_DART_SDK environment variable in Windows to <absolute/path/to/my/dart/sdk>

Unfortunately I've never found a good page on the MS site on how to set environment variables, but there are plenty of unofficial sites with instructions for all versions of Windows.

@kenzieschmoll kenzieschmoll requested a review from a team as a code owner November 1, 2023 22:18
@hannah-hyj
Copy link
Member

CONTRIBUTING.md should also be updated

@DanTup
Copy link
Contributor

DanTup commented Nov 9, 2023

CONTRIBUTING.md should also be updated

Thanks! I'll do this soon - I'm going to need to update the VS Code bits (changes to dart.customDevTools) anyway, but I might make some additional tweaks after #6695 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants