Skip to content
This repository has been archived by the owner on Jan 31, 2023. It is now read-only.

[DO NOT MERGE] Delete smart contracts package #66

Closed

Conversation

Jakeeyturner
Copy link
Contributor

Closes #9

Signed-off-by: Jakeeyturner jaketurner25@live.com

static async showSmartContractPackagesQuickPickBox(prompt: string): Promise<string | undefined> {
const blockchainPackageExplorerProvider = myExtension.getBlockchainPackageExplorerProvider();

const projects: Array<PackageTreeItem> = await blockchainPackageExplorerProvider.getChildren();
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be calling your new function

placeHolder: prompt
};

const projectNames = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a type

}

const blockchainPackageExplorerProvider = myExtension.getBlockchainPackageExplorerProvider();
const packages: Array<PackageTreeItem> = await blockchainPackageExplorerProvider.getChildren();
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be calling this it should use the new function

vscode.window.showErrorMessage('Issue reading smart contract package folder:' + this.packageDir);
return;
}
}
this.tree = await this.createPackageTree(this.packageArray as Array<PackageTreeItem>);
Copy link
Contributor

Choose a reason for hiding this comment

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

this.packageArray isn't an array of packageTreeItems

import * as child_process from 'child_process';
import stripAnsi = require('strip-ansi');

import * as childProcessPromise from 'child-process-promise';
import { ConsoleOutputAdapter } from '../logging/ConsoleOutputAdapter';
import { OutputAdapter } from '../logging/OutputAdapter';
import * as fs from 'fs-extra';
import * as homeDir from 'home-dir';
import { PackageTreeItem } from '../explorer/model/PackageTreeItem';
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't used, if it is and i'm blind you shouldn't be creating packageTreeItems in here it should do this in the packageProvider

@@ -126,4 +128,28 @@ describe('Commands Utility Function Tests', () => {
});
});

describe('getPackages', () => {
// Should move and rewrite aspects of 'getChildren' tests from blockchainPackageExplorer and move them here.
Copy link
Contributor

Choose a reason for hiding this comment

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

do you still need this comment?

await fs.remove(packageDir + '/' + packages[index].name);

}
return blockchainPackageExplorerProvider.refresh();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should call the refresh command eg vscode.commands.execute('blockchainAPackageExplorer.refreshEntry')

@Jakeeyturner Jakeeyturner force-pushed the issue-9-rebase2 branch 5 times, most recently from 1f901e8 to 0b3ae63 Compare September 10, 2018 12:50
packageToDelete = await UserInputUtil.showSmartContractPackagesQuickPickBox('Choose the smart contract package that you want to delete');
}

const packages: any[] = await CommandUtil.getPackages();
Copy link
Contributor

Choose a reason for hiding this comment

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

is the type string here if so we should change it to be string


public static async getPackages(): Promise<string[]> {
console.log('CommandUtils: getPackages');
let packageArray: Array<any> = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

this is an array of strings

cazfletch
cazfletch previously approved these changes Sep 12, 2018
Copy link
Contributor

@cazfletch cazfletch left a comment

Choose a reason for hiding this comment

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

LGTM

Closes IBM-Blockchain#9

Signed-off-by: Jakeeyturner <jaketurner25@live.com>
@Jakeeyturner Jakeeyturner changed the title Delete smart contracts package [DO NOT MERGE] Delete smart contracts package Sep 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants