Skip to content

Conversation

@alexzherdev
Copy link
Contributor

Resolves #156.

@sindresorhus sindresorhus changed the title Add prefer-textcontent rule Add prefer-textcontent rule Jan 30, 2019
@sindresorhus
Copy link
Owner

Hey, thanks for the pull request 🙌 I have gotten a huge response on the IssueHunt bounty issues and I now have a very long backlog of pull requests to review, so it might take some time until I will be able to review this. However, you can help move this along by helping out and reviewing the other submissions. If everyone helps everyone we can manage this 👌.

Some things to look for when reviewing:

  • Ensure the rule addition adheres to the instructions.
  • There are never too many tests. Try to find both passing and failing cases, especially obscure cases.
  • Check the docs for typos and whether something could be more clear.
  • Look at the rule code and see if anything could be simplified or done better. If something is unclear in the code, ask the submitter to explain.

valid: [
'innerText;',
'node.textContent;',
'node[innerText];'
Copy link
Owner

Choose a reason for hiding this comment

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

Can you also add:

  • innerText = true;
  • 'node['innerText'];'

output: 'node.textContent;',
errors: [error]
}
]
Copy link
Owner

Choose a reason for hiding this comment

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

Can you also add an assignment case: node.innerText = 'foo';

@sindresorhus
Copy link
Owner

I just realized that to be consistent with the naming of the other rules, this rule needs to be named prefer-textcontent => prefer-text-content.

@alexzherdev alexzherdev force-pushed the 156-prefer-textcontent branch from 046cfbd to 61da4e4 Compare February 5, 2019 02:00
@alexzherdev alexzherdev changed the title Add prefer-textcontent rule Add prefer-text-content rule Feb 5, 2019
@sindresorhus sindresorhus merged commit c493115 into sindresorhus:master Feb 7, 2019
@sindresorhus
Copy link
Owner

Nice work, @alexzherdev 🙌

@alexzherdev alexzherdev deleted the 156-prefer-textcontent branch April 6, 2019 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants