Skip to content

change test config storage path from extension folder to storage folder#159

Merged
ansyral merged 4 commits intomicrosoft:masterfrom
ansyral:bug154
Apr 3, 2018
Merged

change test config storage path from extension folder to storage folder#159
ansyral merged 4 commits intomicrosoft:masterfrom
ansyral:bug154

Conversation

@ansyral
Copy link
Collaborator

@ansyral ansyral commented Mar 30, 2018

Signed-off-by: xuzho xuzho@microsoft.com

to fix #154 #153

@ansyral ansyral requested a review from yaohaizh March 30, 2018 03:57
constructor(private readonly _configPath: string, private readonly _projectManager: ProjectManager) {
private readonly _configPath: string;
constructor(storagePath: string, private readonly _projectManager: ProjectManager) {
this._configPath = path.join(storagePath, 'configs', Configs.TEST_LAUNCH_CONFIG_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

If make this config file locate at the project specific user folder, how can user make changes to this file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we provide command Java: Edit Test Configuration for user to open config, but this is a good suggestion, I would move the config to .vscode folder where user could directly access it.

Copy link
Contributor

@yaohaizh yaohaizh left a comment

Choose a reason for hiding this comment

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

Why change the location of configuration file?

ansyral added 3 commits April 3, 2018 11:23
Signed-off-by: xuzho <xuzho@microsoft.com>
…uration

Signed-off-by: xuzho <xuzho@microsoft.com>
try {
config = await configManager.loadConfig();
} catch (ex) {
window.showErrorMessage('Failed to load test config! Please check whether your test configuration is a valid JSON file');
Copy link
Contributor

Choose a reason for hiding this comment

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

the test config. Same below

if (!workspaceFolders) {
throw new Error('Not supported without a folder!');
}
this._configPath = path.join(workspaceFolders[0].uri.fsPath, '.vscode', Configs.TEST_LAUNCH_CONFIG_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

How about multiple root scenario? Always pick up the first one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes , I would always store it in the first one considering the config for different projects are merged in one config

private createTestConfigIfNotExisted(): Promise<void> {
return new Promise((resolve, reject) => {
mkdirp(path.dirname(this._configPath), (merr) => {
if (merr && merr.code !== 'EEXIST') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Storage path should be File path ?

Signed-off-by: xuzho <xuzho@microsoft.com>
@ansyral ansyral merged commit d6c0b23 into microsoft:master Apr 3, 2018
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.

Config file can't auto-generate project configuration information

2 participants