-
Notifications
You must be signed in to change notification settings - Fork 885
Conversation
cc @ScottSWu |
rationale?: string; | ||
} | ||
|
||
export type RuleType = "functionality" | "maintainability" | "style" | "typescript"; |
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.
I'm not sure we use string literal union types anywhere else, and this might break things for 1.7 consumers. I feel okay with that at this point because 1.8 has been out for awhile, but we could use a different tool instead of string literal union types if we need to
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.
yeah this seems fine
Ready for review! |
for (const key of Object.keys(metadata)) { | ||
yamlData[key] = (<any> metadata)[key]; | ||
} | ||
yamlData.optionsJSON = JSON.stringify(metadata.options, undefined, " "); |
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.
minor: the last arg could be the number literal 2
; I think that's slightly more standard (?)
(here and above, L65)
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.
Good call, much easier than guessing how many spaces are in a string
@@ -99,6 +111,12 @@ module.exports = function (grunt) { | |||
"mochaTest", | |||
"run:testRules", | |||
].concat(checkBinTest)); | |||
grunt.registerTask("docs", [ |
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.
this should probably be run as part of the CI build.
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.
Agreed, it can run when a tag is created for a non-dev release. People will still be able to do this manually though if they update other parts of the docs site that aren't generated via this task.
Can we automatically push to our repo from CircleCI?
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.
It should be run as part of our verification step on every commit -- we want to make sure the docs TS code doesn't get broken
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.
I understand what you're getting at: we want to make sure that docs.ts
builds and passes the linter. However, we don't want the part of the task to run that actually updates the docs site, because the docs site would advertise unreleased TSLint rules and options.
For simplicity for now, I'll adjust things so that the build part of the current docs task is run on every build, including CircleCI builds. The actual updating of the files in the tslint-gh-pages, that is, the execution of buildDocs.js
, will still have to be done manually for the time being
mostly minor comments; 👎 for not running the new task as part of CI build |
Also fix lint errors in buildDocs.ts
lgtm 👍 |
Sweet, let's merge this |
* Adds metadata for each rule * Adds a gulp docs command that runs a script which reads the metadata from each rule and outputs file into the Jekyll docs site * Removes docs/sample.tslint.json because it's redundant documentation and also to make room for the docs build script.
Pushing up my work on metadata for rules. This metadata is currently used to generate large parts of http://palantir.github.io/tslint/.
Metadata contains:
This PR:
gulp docs
command that runs a script which reads the metadata from each rule and outputs file into the Jekyll docs sitedocs/sample.tslint.json
because it's redundant documentation and also to make room for the docs build script.TODO: