runner.ts: Break into separate functions #2572
Conversation
andy-hanson
force-pushed
the
runner
branch
2 times, most recently
from
03a7abe
to
2b42f95
Compare
src/runner.ts
Outdated
| if (this.options.version) { | ||
| this.outputStream.write(Linter.VERSION + "\n"); | ||
| return onComplete(0); | ||
| export async function run(options: Options, outputStream: NodeJS.WritableStream): Promise<Status> { |
There was a problem hiding this comment.
While you're in the area, would it make sense to stop using any console messages at all? In addition to passing the CLI options and an outputStream it makes sense to also pass an errorStream.
|
|
||
| export interface IRunnerOptions { | ||
| export interface Options { |
There was a problem hiding this comment.
breaking. Can you rename back?
src/runner.ts
Outdated
| if (this.options.version) { | ||
| this.outputStream.write(Linter.VERSION + "\n"); | ||
| return onComplete(0); | ||
| export async function run(options: Options, outputStream: NodeJS.WritableStream, errorStream: NodeJS.WritableStream): Promise<Status> { |
There was a problem hiding this comment.
to avoid a breaking change and waiting for 6.0, can you put a Runner class as a wrapper? We can remove it in the next major rev
There was a problem hiding this comment.
According to #2446 (comment), anything not in index is not public.
|
ping here, can you address @nchen63's comments? |
|
According to #2446 (comment), anything not in |
|
fair enough; I assumed it was exported through index. got a few merge conflicts left though |
|
new conflicts with 2bc8982 :( |
PR checklist
Overview of change:
Just a refactor. Breaks apart the class into smaller functions.
Converted it to return a Promise, but the implementation doesn't use Promises yet.