New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update: Check strings in RegExp constructors for no-regex-spaces
(fixes #3586)
#6379
Update: Check strings in RegExp constructors for no-regex-spaces
(fixes #3586)
#6379
Conversation
By analyzing the blame information on this pull request, we identified @vitorbal, @pedrottimark and @alberto to be potential reviewers |
Thanks for the pull request, @jacksonrayhamilton! I took a look to make sure it's ready for merging and found some changes are needed:
Can you please update the pull request to address these? (More information can be found in our pull request guide.) |
Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA). 📝 Please visit http://contribute.jquery.org/CLA/ to sign. After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know. If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check. |
LGTM |
* @returns {void} | ||
* @private | ||
*/ | ||
function regexCheck(node, value) { |
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.
Can you rename each function to begin with "check"? Our conventions always have a verb as the first word in a function name.
The code looks read good, just one convention nitpick and otherwise lgtm. @IanVS want to take a look? |
LGTM, thanks @jacksonrayhamilton! |
LGTM |
Fixed the issue, glad to contribute. |
Rule doc looks good to me |
Just holding off on merging until we're sure we don't need another patch release. |
Thanks @jacksonrayhamilton ! |
Fixes #3586, making it so a single string literal in a
RegExp
constructor is also checked for multiple consecutive spaces.