moodle-plugin-ci icon indicating copy to clipboard operation
moodle-plugin-ci copied to clipboard

Add "Javascript coding style problems" from moodle.org/plugins prechecker to moodle-plugin-ci

Open abias opened this issue 5 years ago • 7 comments

Hi,

I have noticed that the moodle.org/plugins prechecker runs some check on the Javascript Coding style which are currently not reported by moodle-plugin-ci.

For an example, please see https://moodle.org/plugins/pluginversion.php?id=19343&smurf=html#js where this error is reported:

lib/editor/atto/plugins/styles/yui/src/button/js/button.js
(#37) Expected space or tab after '/*' in comment. (spaced-comment)

At the same time, the corresponding Job in Travis CI does not report any problems with JavaScript: https://travis-ci.org/moodleuulm/moodle-atto_styles/jobs/518201841

Do you have an idea if these JavaScript checks can also be added to moodle-plugin-ci?

Cheers, Alex

abias avatar Jun 12 '19 07:06 abias

Hi @polothy ,

may I ping you if you have an idea about this gap?

Thanks, Alex

abias avatar Nov 29 '19 06:11 abias

Hrm... I don't know what the js check is. IIRC, it would be defined somewhere in this project: https://github.com/moodlehq/moodle-local_ci

polothy avatar Dec 02 '19 18:12 polothy

Thank you @polothy .

I have raised this question in Moodle Dev Chat to see if anyone from Moodle HQ can shed a light on this architecture.

abias avatar Dec 04 '19 07:12 abias

It's an eslint warning which matches https://eslint.org/docs/rules/spaced-comment

The line in question is:

/*global rangy*/

You can fix this in several ways:

  1. Switch from rangy to window.rangy - this is the one that you should do. This is more correct. rangy is just a shortcut to window.rangy. The rangy option works because it's the default scope for when an object is not properly scoped.
  2. Switch the line to read /* global rangy */

It's a valid issue, not a bug in the checker. You can see the same thing by running eslint manually:

npx eslint lib/editor/atto/plugins/styles/yui/src/button/js/button.js

andrewnicols avatar Dec 04 '19 08:12 andrewnicols

Thanks, @andrewnicols , for your quick feedback about the underlying coding style problem. I will use your improvement proposal soon to improve the code.

However, my question was about the fact that this coding style problem is reported by the moodle.org/plugins prechecker but not by Travis CI which is running local_codechecker.

@polothy was wondering if these eslint checks are only performed by moodle-local_ci in the Moodle plugins repository.

Do you also have any insights about that for me?

Thanks, Alex

abias avatar Dec 04 '19 10:12 abias

Hey @andrewnicols , may I ping you again about the pending question from my comment above?

@polothy was wondering if these eslint checks are only performed by moodle-local_ci in the Moodle plugins repository.

Thanks, Alex

abias avatar Feb 13 '20 19:02 abias

hi! I added an option to run grunt with arguments to show/fail on lint warnings, see https://github.com/blackboard-open-source/moodle-plugin-ci/pull/111 it is already merged but not tagged yet

marinaglancy avatar May 23 '20 16:05 marinaglancy