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

Fixing the version output and adding tests #82

Merged
merged 2 commits into from
Feb 2, 2017
Merged

Conversation

jpricket
Copy link
Member

@jpricket jpricket commented Feb 1, 2017

No description provided.

@msftclas
Copy link

msftclas commented Feb 1, 2017

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;


// Ignore WARNings that may be above the desired lines
if (skipWarnings) {
let index = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd love me a type on this

@@ -49,7 +50,7 @@ export class FindWorkspace implements ITfvcCommand<IWorkspace> {
}

// Find the workspace name and collectionUrl
const lines = stdout.replace("\r\n", "\n").split("\n");
const lines = CommandHelper.SplitIntoLines(stdout);
Copy link
Contributor

Choose a reason for hiding this comment

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

type here too?

@@ -23,8 +24,12 @@ export class GetVersion implements ITfvcCommand<string> {
}

public async ParseOutput(executionResult: IExecutionResult): Promise<string> {
const stdout = executionResult.stdout;
const lines = CommandHelper.SplitIntoLines(executionResult.stdout);
Copy link
Contributor

Choose a reason for hiding this comment

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

type-ola? (missed it last time)

assert.equal(lines[4], "");
});

it("should verify SplitIntoLines - leave WARNings", function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a test where there's a non-WARN line between two WARN lines? Is that a possible scenario?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not that I have seen WARNings all come before the actual output of the command. So we simply strip them from the top.

@jeffyoung jeffyoung merged commit 9124d38 into master Feb 2, 2017
@jpricket jpricket deleted the users/jpricket/0201 branch February 17, 2017 15:25
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