Missed a couple mocked commands. Bump patch version #334
Conversation
| @@ -1,5 +1,5 @@ | |||
| import path = require('path'); | |||
| import fs = require('fs'); | |||
| import * as path from 'path'; | |||
There was a problem hiding this comment.
import ... require has slightly different semantics than import ... from. My understanding is that require executes the module as the CommonJS and AMD require function does so that any side effects from the module are applied. import ... from is the normal way to import a module in TypeScript.
node/mock-answer.ts
Outdated
| which?: { [key: string]: string }, | ||
| } | ||
|
|
||
| // TODO TypeScript 2.1: use `keyof` |
There was a problem hiding this comment.
I believe we're using version 1.8.7 of the TypeScript compiler. TypeScript 2.1 adds a keyof operator: you can just say keyof TaskLibAnswer instead of defining MockedCommand.
node/mock-answer.ts
Outdated
| } | ||
|
|
||
| // TODO TypeScript 2.1: use `keyof` | ||
| export type MockedCommand = "checkPath" |
There was a problem hiding this comment.
Double quotes here and single below?
| } | ||
|
|
||
| public getResponse(cmd: string, key: string, debug: (message: string) => void): any { | ||
| public getResponse(cmd: MockedCommand, key: string, debug: (message: string) => void): any { |
There was a problem hiding this comment.
Breaking change? May want to bump at least the minor since it's mock and not the main code.
There was a problem hiding this comment.
Yeah, it's technically a breaking change if people are calling getResponse directly for some reason. There aren't any uses right now in vsts-tasks -- it's just used internally in mock-task and mock-toolrunner.
I said MockedCommand since it's "one of the commands that are mocked" and not the mock itself.
There was a problem hiding this comment.
There might be 3rd party extensions using it
There was a problem hiding this comment.
Yeah, we can bump the minor version
|
LGTM. One last thing: @bryanmacfarlane Can you confirm that bumping minor version is sufficient when making a breaking change? Don't mean to be pedantic but want to make sure we are staying consistent with the versioning for this lib. Especially given what @madhurig helpfully mentioned about 3rd party extensions. |
|
fine. |
* Missed a couple commands. Catch unmocked commands at compile time. Bump patch version * Need to bump again, had un-synced changes upstream * Strings should be single-quoted * Bump minor version * clarify TODO
4 participants
Forgot to bump the patch version before.
Noticed that I missed a couple of commands that were actually mocked, and so the previous changes were insufficient to stop coercing the
TaskLibAnswertype in our existing code. I defined theMockedCommandtype here so that the compiler will catch all arguments togetResponse.