Skip to content

Conversation

@MrHen
Copy link
Contributor

@MrHen MrHen commented Apr 17, 2019

Fixes #251.

This is an initial PR to judge interest. As I mentioned in the issue, I was planning on writing this rule anyway and figured I would submit it to unicorn instead of building a one-off package.

I don't expect this to be a popular rule -- it's merely solving a nitpick I've had on my existing team's code style.

There is some remaining work to do but I didn't want to polish it without knowing if the PR would be accepted.

  • Document configuration options
  • Update rule listing in readme
  • Update recommended config
  • Update recommended config in readme
  • Resolve TODOs (mostly missing tests)
  • Split up the Identifier function into smaller, easier to digest chunks

@sindresorhus
Copy link
Owner

While I won't use this rule personally, I'm happy to host it here if you can commit to maintain it. I think it should be "off" by default in the recommended config though.

@MrHen
Copy link
Contributor Author

MrHen commented Jun 22, 2019

@sindresorhus This PR is now ready for review.

Relatedly, I'm not sure what your preferred process is for reviewing / merging PRs on your repositories. I've been assuming you want to review every PR and will decide when to merge them. Is that correct?

@sindresorhus
Copy link
Owner

Relatedly, I'm not sure what your preferred process is for reviewing / merging PRs on your repositories. I've been assuming you want to review every PR and will decide when to merge them. Is that correct?

It doesn't have to be me, but it's always useful to get a second set of eyes on the code.

Copy link
Owner

@sindresorhus sindresorhus left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'm gonna leave this open for a day to give other people a chance to review if they want.

@sindresorhus sindresorhus merged commit 1982af5 into sindresorhus:master Jun 24, 2019
@MrHen MrHen deleted the feature/251-Forbid-New branch June 24, 2019 12:49
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.

Rule Proposal: Forbid naming variables beginning with "new"

2 participants