Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

586 walk in new rule template #702

Merged
merged 4 commits into from
Dec 28, 2018
Merged

586 walk in new rule template #702

merged 4 commits into from
Dec 28, 2018

Conversation

reduckted
Copy link
Contributor

PR checklist

Overview of change:

I've changed the new rule template to use a walk function instead of a class extending RuleWalker.

When you use a walk function, there's a bit of a difference between a rule with options and one without options (compared to using a RuleWalker), so I've added an additional prompt to ask you if the rule will have options (defaulting to false, because most rules don't have options). The answer to this prompt will change the generated rule slightly.

When the rule has options:

  • An empty Options interface is defined, ready to be filled in.
  • The options and optionsDescription metadata properties are added with TODO comments.
  • The optionExamples metadata property is included.
  • The applyWithFunction function is called with the rule's options.
  • The WalkContext is defined as WalkContext<Options>.

When the rule does not have options:

  • The options and optionsDescription metadata properties are added with blank values and do not need to be changed.
  • The optionExamples metadata property is excluded.
  • The applyWithFunction function is called without the rule's options.
  • The WalkContext is defined as WalkContext<void>.

Since this required a few conditional sections in the template, I've changed the template to use EJS. instead of just being an interpolated string (docs for EJS can be found here).

Is there anything you'd like reviewers to focus on?

Nope.

Copy link

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

LGTM but for the templating engines comment; marking a request for changes just to get it answered 😉 but I'm ok with merging as in otherwise. Good stuff!

build-tasks/create-rule.js Show resolved Hide resolved
@JoshuaKGoldberg JoshuaKGoldberg added the PR: Waiting for Author Changes have been requested that the pull request author should address. label Dec 28, 2018
Copy link

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@JoshuaKGoldberg JoshuaKGoldberg merged commit a065198 into microsoft:master Dec 28, 2018
@JoshuaKGoldberg JoshuaKGoldberg removed the PR: Waiting for Author Changes have been requested that the pull request author should address. label Dec 28, 2018
@reduckted reduckted deleted the 586-walk-in-new-rule-template branch December 28, 2018 03:33
@IllusionMH IllusionMH added this to the 6.1.0-beta milestone Feb 19, 2019
apawast pushed a commit to lupine86/tslint-microsoft-contrib that referenced this pull request Feb 26, 2019
* Used a walk function instead of extending RuleWalker in the new rule template.

* Used EJS for the new rule template.

* Added a prompt asking whether the new rule has options.

* Replaced EJS templates with Underscore templates.
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.

Use a walk function instead of extending AbstractRule in our new-rule templates
3 participants