Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tslint in ghooks coverage should be increased #15263

Closed
Tyriar opened this issue Nov 9, 2016 · 4 comments
Closed

tslint in ghooks coverage should be increased #15263

Tyriar opened this issue Nov 9, 2016 · 4 comments
Assignees
@bpasero
Labels
debt Code quality issues
Milestone
November 2016

Comments

@Tyriar
Copy link
Member

Tyriar commented Nov 9, 2016 •
edited

It appears that there is an implicit expectation that the tslint extension is used for the codebase, we should increase coverage of the tslint filter in ghooks to cover all cases we want as not all contributors will use the extension.

const tslintFilter = [
	'src/**/*.ts',
	'extensions/**/*.ts',
	'!**/*.d.ts',
	'!**/typings/**',
	'!src/vs/base/**/*.test.ts', // < Why is this here?
	'!extensions/typescript/test/colorize-fixtures/**', // < Why is this here?
	'!extensions/vscode-api-tests/testWorkspace/**',
	'!src/vs/workbench/**/*.test.ts', // < Why is this here?
	'!extensions/**/*.test.ts' // < Why is this here?
];

I also suggest that in the future any exceptions to the rules have comments with the reasoning behind the decision.

Related fix for filters not being restored #15259
@Tyriar Tyriar added the debt Code quality issues label Nov 9, 2016
@Tyriar Tyriar mentioned this issue Nov 9, 2016
Merged
@Tyriar
Copy link
Member Author

Tyriar commented Nov 9, 2016

/cc @bpasero

@joaomoreno
Copy link
Member

joaomoreno commented Nov 10, 2016 •
edited

Long story short: no one cares enough when it comes to linting tests. That was the overall opinion when tslint was introduced into the project. People just didn't want to get bothered with all those warnings at once. This issue should be broken down into many issues, across everyone's code, so everyone takes care of their own turf.

@Microsoft/vscode

@joaomoreno joaomoreno added this to the Backlog milestone Nov 10, 2016
@bpasero bpasero modified the milestones: November 2016, Backlog Nov 10, 2016
@bpasero bpasero assigned bpasero and unassigned joaomoreno Nov 10, 2016
@alexdima
Copy link
Member

@joaomoreno @jrieken I'd like for code in vs/editor/test to get linted, formatted, etc, i.e. apply all the rules of vs/editor/common to it. What do I need to do?

@joaomoreno
Copy link
Member

@alexandrudima They are already under hygiene: d5e8fcf

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bpasero bpasero

Labels
debt Code quality issues
Projects
None yet
Milestone
November 2016
Development

No branches or pull requests
4 participants
@joaomoreno @bpasero @Tyriar @alexdima