-
Notifications
You must be signed in to change notification settings - Fork 31
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
Build: Update dependencies #81
Conversation
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.
+1 by reading
|
||
exports.syntaxHighlight = syntaxHighlight; | ||
|
||
exports.postPreprocessors = { | ||
_default: function( post, fileName, callback ) { | ||
_default( post, _fileName, callback ) { |
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.
Did you add this underscore on purpose?
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.
Yes, our ESLint config requires all variables & function parameters to be used inside of the function body. For parameters it's not possible in 100% of cases and then prefixing with an underscore is an opt-out.
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.
We should probably use after-used
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.
We had a discussion about that some time ago and we decided to use all
as there are more cases where we'd leave internal unused parameters than in APIs where they have to be there. This applies especially to AMD module definitions where we've been constantly missing unused dependencies.
We had to prefix some parameters with _
in Core because of this rule but there weren't too many of them.
jQuery
|
.eslintignore
Outdated
@@ -0,0 +1 @@ | |||
node_modules/** |
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.
This should be redundant afaik.
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.
Hmm, I guess I can remove the whole file then.
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.
Done.
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.
LGTM
@Krinkle the convention for the
not:
See https://contribute.jquery.org/commits-and-pull-requests/#commit-guidelines |
This makes all dependencies up to date and switches the project from JSHint to ESLint. That resolves most reported security issues.
A significant number of changes had to be made to
jquery-wp-content
to make it compatible with the new highlight.js; that PR needs to land first.Ref jquery/jquery-wp-content#430