Skip to content

Lint unit tests via harnessSources in Jakefile.js#8785

Merged
sandersn merged 4 commits into
masterfrom
lint-unit-tests
May 24, 2016
Merged

Lint unit tests via harnessSources in Jakefile.js#8785
sandersn merged 4 commits into
masterfrom
lint-unit-tests

Conversation

@sandersn
Copy link
Copy Markdown
Member

@sandersn sandersn commented May 24, 2016

A couple of preprocessing tests needed to change because of deleting trailing whitespace. Otherwise the changes are straightforward.

Fixes #8763

@sandersn
Copy link
Copy Markdown
Member Author

@Andy-MS can you take a look?

Comment thread Jakefile.js
});
var lintTargets = compilerSources
.concat(harnessCoreSources)
.concat(harnessSources)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't need client.ts added manually below, that's already in harnessSources.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the reminder. fixed.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented May 24, 2016

👍

///<reference path='..\..\..\..\src\harness\harness.ts' />

describe("DocumentRegistry", () => {
/* tslint:disable no-unused-variable */
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should there be an issue reminding us to go and fix things like this later?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, these variables are intentionally unused because they are testing state sharing between instances.

@ghost
Copy link
Copy Markdown

ghost commented May 24, 2016

Printing lintTargets shows some duplicates. This could make linting take unnecessarily long. Could we just add a step to remove duplicates?

@sandersn
Copy link
Copy Markdown
Member Author

sandersn commented May 24, 2016

I'm not sure of the correct Javascript way to do this.

Gets rid of 3 dupes: protocol.d.ts, session.ts and editorServices.ts.

@ghost
Copy link
Copy Markdown

ghost commented May 24, 2016

👍

@sandersn sandersn merged commit c09793d into master May 24, 2016
@sandersn sandersn deleted the lint-unit-tests branch May 24, 2016 21:56
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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.

3 participants