-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
@sushain97 your build status isn't reporting -- have you enabled CircleCI on your fork? |
@adidahiya I did (https://circleci.com/gh/sushain97/tslint-blueprint/1?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link) but for some reason the branch didn't build... |
src/rules/blueprintClassNameRule.ts
Outdated
return ts.forEachChild(ctx.sourceFile, callback); | ||
|
||
function callback(node: ts.Node): void { | ||
if (node.kind === ts.SyntaxKind.StringLiteral && (node as ts.StringLiteral).text.startsWith("pt-")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some suggestions on the string test:
- allow the exact literal
"pt-"
to go through without a lint failure. it's not a valid blueprint class name and would be a false positive if we linted for it - check for ".pt-" as well, so we can lint things like
document.querySelector(".pt-button")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with tests. Seems to actually be building now as well: https://circleci.com/gh/sushain97/tslint-blueprint/2?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(thanks for the suggestions, those cases didn't cross my mind)
975c276
to
723be2c
Compare
We error on usage of string literal Blueprint class names in order to minimize typos and ensure compile-time correctness of class references.