Content-Length: 451795 | pFad | https://github.com/SSilence/selfoss/pull/951

ECF lint JS with ESlint by niol · Pull Request #951 · fossar/selfoss · GitHub
Skip to content
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

lint JS with ESlint #951

Merged
merged 1 commit into from
Jun 27, 2017
Merged

lint JS with ESlint #951

merged 1 commit into from
Jun 27, 2017

Conversation

niol
Copy link
Collaborator

@niol niol commented Jun 20, 2017

No description provided.

.eslintrc.json Outdated
"keyword-spacing": "error",
"linebreak-style": ["error", "unix"],
"no-multiple-empty-lines": "error",
"no-plusplus": "error",
Copy link
Member

Choose a reason for hiding this comment

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

I find this rule weird but that is probably just a habit so I do not oppose it. In either case i += 1 would be preferred replacement to i = i + 1.

gruntfile.js Outdated
@@ -157,4 +162,5 @@ module.exports = function(grunt) {
grunt.registerTask('default', ['install', 'versionupdater', 'compress']);
grunt.registerTask('version', ['versionupdater']);
grunt.registerTask('zip', ['compress']);
grunt.registerTask('lint_js', 'Check JS syntax', ['eslint']);
Copy link
Member

Choose a reason for hiding this comment

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

Few lines above, we use client:install. Not sure if it is recommended practice but we should probably be consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean I should rename the task client:lint?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe, I am not sure if the convention is correct.

package.json Outdated
"grunt-composer": "^0.4.5",
"grunt-contrib-compress": "^0.10.0",
"grunt-eslint": "",
Copy link
Member

Choose a reason for hiding this comment

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

Why is there an empty constraint?

{
"root": true,
"extends": "eslint:recommended",
"rules": {
Copy link
Member

Choose a reason for hiding this comment

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

Going through the list, I would also add at least these rules:

{
    "brace-style": "error",
    "camelcase": "error",
    "curly": "error",
    "eol-last": "error",
    "no-use-before-define": "error",
    "quotes": ["error", "single"],
    "unicode-bom": "error"
}

gruntfile.js Outdated
@@ -157,4 +162,5 @@ module.exports = function(grunt) {
grunt.registerTask('default', ['install', 'versionupdater', 'compress']);
grunt.registerTask('version', ['versionupdater']);
grunt.registerTask('zip', ['compress']);
grunt.registerTask('client:lint', 'Check JS syntax', ['eslint']);
Copy link
Member

Choose a reason for hiding this comment

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

Apparently, the convention is task:target, that means lint:client.

https://gruntjs.com/configuring-tasks#task-configuration-and-targets

@@ -109,16 +109,17 @@ var selfoss = {
*/
getValues: function(element) {
var values = {};

$(element).find(':input').each(function (i, el) {
Copy link
Member

Choose a reason for hiding this comment

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

Add "space-before-function-paren": ["error", "never"]

http://eslint.org/docs/rules/space-before-function-paren

@@ -128,7 +129,7 @@ var selfoss = {

setSession: function() {
Cookies.set('onlineSession', 'true', {expires: 10,
Copy link
Member

Choose a reason for hiding this comment

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

Add "object-curly-newline": ["error", {"multiline": true, "consistent": true}]

http://eslint.org/docs/rules/object-curly-newline

@@ -202,46 +204,49 @@ var selfoss = {
showErrors: function(form, errors) {
$(form).find('span.error').remove();
$.each(errors, function(key, val) {
form.find("[name='"+key+"']").addClass('error').parent('li').append('<span class="error">'+val+'</span>');
form.find('[name=\'' + key + '\']').addClass('error').parent('li').append('<span class="error">' + val + '</span>');
Copy link
Member

Choose a reason for hiding this comment

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

You can use double quotes instead of the escaped single quotes.


if (selfoss.isSmartphone()) {
$('.entry-icon, .entry-datetime').unbind('click').click(function(e) {
e.preventDefault(); return false;
Copy link
Member

Choose a reason for hiding this comment

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

Add max-statements-per-line: ["error", { "max": 1 }]

http://eslint.org/docs/rules/max-statements-per-line

var tagsCountEl = $('#nav-tags > li > span.tag').filter(function(i){
return $(this).html()==tag; }

var tagsCountEl = $('#nav-tags > li > span.tag').filter(function(){
Copy link
Member

Choose a reason for hiding this comment

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

Add "space-before-blocks": "error"

http://eslint.org/docs/rules/space-before-blocks


var tagsCountEl = $('#nav-tags > li > span.tag').filter(function(){
return $(this).html() == tag;
}
).next();
Copy link
Member

Choose a reason for hiding this comment

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

This should be on the same line as the closing bracket, though I could not find an eslint rule.

}

if (unread) {
unreadstats = unreadstats - 1;
Copy link
Member

Choose a reason for hiding this comment

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

Do we still do the explicit (inc|dec)rements?

// hide input form
parent.find('.source-edit-form').hide();

// update title
var title = $('<p>').html(response.title).text();
parent.find('.source-title').text(title);
parent.find("input[name='title']").val(title)
parent.find('input[name=\'title\']').val(title);
Copy link
Member

Choose a reason for hiding this comment

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

Replace the escaped single quotes with double quotes.

@jtojnar jtojnar merged commit e55ef1b into fossar:master Jun 27, 2017
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.

2 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/SSilence/selfoss/pull/951

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy