-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add more rules to dangerfile #1703
Conversation
}); | ||
|
||
all_files.forEach(function(file) { | ||
if (/^services\/.+\/.+\.js$/.test(file) && file.endsWith('.js') && !file.endsWith('.tester.js')) { |
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.
You should be able to get away with just using this regex:
if (/^services\/.+\/[^\.]+(?:(?!\.tester))\.js$/.test(file)) {
But the way you have it currently is more self explanatory.
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 think in this case the longer expression is more readable
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 think we can merge it and then prepare a PR which will test if rules works as expected.
danger.js
to the latest version (includes fixes for Unexpected behaviour calling diffForFile on added or renamed file danger/danger-js#532 and diffForFile produces unexpected result if PR is made from fork danger/danger-js#531 )assert
, suggestexpect
syntax (refs Add more checks to dangerfile.js #1553 ).