Skip to content

Fix for Issue #21512 and code cleanup for Jakefile.js#21513

Merged
mhegazy merged 2 commits into
microsoft:masterfrom
TravCav:master
Jan 31, 2018
Merged

Fix for Issue #21512 and code cleanup for Jakefile.js#21513
mhegazy merged 2 commits into
microsoft:masterfrom
TravCav:master

Conversation

@TravCav
Copy link
Copy Markdown
Contributor

@TravCav TravCav commented Jan 31, 2018

Mostly to fix Issue #21512

Jakefile.js was throwing lint warnings so I cleaned some of them up.

@TravCav
Code cleanup for Jakefile.js
5e4df5f 
Jakefile.js was throwing lint warnings so I cleaned some of them up.
mhegazy
mhegazy reviewed Jan 31, 2018
View reviewed changes
Comment thread Jakefile.js Outdated
@@ -1,5 +1,6 @@
// This file contains the build logic for the public repo
// @ts-check
/*jshint esversion: 6 */
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy Jan 31, 2018

Choose a reason for hiding this comment

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

can we not include this line?

Copy link
Copy Markdown
Contributor Author

@TravCav TravCav Jan 31, 2018

Choose a reason for hiding this comment

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

Can we include a .jshintrc file?

@TravCav
took out jshint flag
360bcea 
removed /*jshint esversion: 6 */
@mhegazy mhegazy merged commit d0ab164 into microsoft:master Jan 31, 2018
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Jan 31, 2018

thanks!

@amcasey
Copy link
Copy Markdown
Member

amcasey commented Feb 13, 2018

This change seems to prevent jake lint from reporting any lint errors. Here's a run followed by an invocation of the command it prints out:

e:\ts_gh>jake lint
Linting: node node_modules/tslint/bin/tslint Gulpfile.ts scripts/generateLocalizedDiagnosticMessages.ts "scripts/tslint/**/*.ts" "src/**/*.ts" --exclude "src/lib/*.d.ts" --formatters-dir ./built/local/tslint/formatters --format autolinkableStylish

e:\ts_gh>node node_modules/tslint/bin/tslint Gulpfile.ts scripts/generateLocalizedDiagnosticMessages.ts "scripts/tslint/**/*.ts" "src/**/*.ts" --exclude "src/lib/*.d.ts" --formatters-dir ./built/local/tslint/formatters --format autolinkableStylish

src/services/utilities.ts
ERROR: src/services/utilities.ts:1375:51 semicolon Missing semicolon

@amcasey
Copy link
Copy Markdown
Member

amcasey commented Feb 13, 2018

I'm wondering if there's actually an underlying tslint issue, since generateLocalizedDiagnosticMessages.ts does exist. Amusingly, an easy workaround for this issue is to change 'scripts/generateLocalizedDiagnosticMessages.ts' to 'scripts/*generateLocalizedDiagnosticMessages.ts'. The wildcard bypasses the existence check and then the file is found when linting is actually performed (confirmed by introducing an error into that file and running jake lint).

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@mhegazy mhegazy mhegazy left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TravCav @mhegazy @amcasey