Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

TFVC command framework in place #66

Merged
merged 3 commits into from
Jan 26, 2017
Merged

TFVC command framework in place #66

merged 3 commits into from
Jan 26, 2017

Conversation

jpricket
Copy link
Member

User Story #884931

@msftclas
Copy link

Hi @jpricketMSFT, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Jason Prickett). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

if (this.ensureInitialized()) {
try {
let tfvc: Tfvc = new Tfvc();
let repo: Repository = tfvc.open("D:\\tmp\\tfsTest03_44");
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be great to remove the hard-coded paths.

* Create an instance of this class to build up the arguments that should be passed to the command line.
*/
export class ArgumentBuilder {
private arguments: string[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

I typically prefix private member variables with underscores. Your call.

this._localPath = localPath;
}

getArguments(): string[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

I start public methods with a capital letter (private methods start with lowercase). I'm also typically explicit regarding private v public.

return workspace;
}

private getValue(line: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment would be great as to what the incoming line looks like (presumably there's a ':' somewhere :-).

this.tfvcPath = localPath;
} else {
// TODO get it from settings
this.tfvcPath = "D:\\tmp\\bin\\TEE-CLC-14.0.4\\tf.cmd";
Copy link
Contributor

Choose a reason for hiding this comment

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

hard-coded

removed hard coded paths,
corrected cases on method names,
explicitely declared methods as private/public.
Copy link
Contributor

@jeffyoung jeffyoung left a comment

Choose a reason for hiding this comment

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

LGTM, justadd a TODO for the TfvcError.message

@jpricket jpricket merged commit b2be7f9 into master Jan 26, 2017
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

3 participants