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

[qa] Added linting of JS and CSS #173 #202

Merged
merged 12 commits into from Jun 8, 2021
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Expand Up @@ -42,6 +42,9 @@ jobs:
run: |
pip install -U pip wheel setuptools
pip install -U -r requirements-test.txt

- name: Install npm dependencies
run: sudo npm install -g jshint stylelint

- name: Install openwisp-utils
run: |
Expand Down
21 changes: 21 additions & 0 deletions .jshintrc
@@ -0,0 +1,21 @@
{
"unused": true,
"esversion": 6,
"curly": true,
"strict": "global",
"globals": {
"django": true,
"gettext": true,
"ReconnectingWebSocket": true,
"notificationApiHost": true,
"notificationSound": true,
"notificationSocket": true,
"owNotifyObjectId": true,
"owNotifyAppLabel": true,
"owNotifyModelName": true,
"owIsChangeForm": true,
"getAbsoluteUrl": true,
"dateTimeStampToDateTimeLocaleString": true
},
"browser": true
}
21 changes: 21 additions & 0 deletions .stylelintrc.json
@@ -0,0 +1,21 @@
{
"rules": {
"block-no-empty": null,
"color-no-invalid-hex": true,
"comment-empty-line-before": ["always", {
"ignore": ["stylelint-commands", "after-comment"]
}],
"declaration-colon-space-after": "always",
"indentation": [2, {
"except": ["value"]
}],
"max-empty-lines": 4,
"rule-empty-line-before": ["never-multi-line", {
"except": ["first-nested"],
"ignore": ["after-comment", "inside-block"]
}],
"unit-allowed-list": ["em", "rem", "%", "s", "px", "vh", "deg"],
"declaration-block-trailing-semicolon": "always",
"property-no-unknown": true
}
}
20 changes: 20 additions & 0 deletions README.rst
Expand Up @@ -814,6 +814,7 @@ automated builds of different OpenWISP modules.
^^^^^^^^^^^^^^^^^^^^^^

Shell script to automatically format Python code. It runs ``isort`` and ``black``.
It also runs ``stylelint`` to format CSS code.
Copy link
Member

Choose a reason for hiding this comment

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

Shell script to automatically format Python code. It runs isort. black and stylelint.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think its better.
Shell script to automatically format Python and CSS code. It runs isort. black and stylelint.

Copy link
Member

Choose a reason for hiding this comment

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

Please mention that the command won't fail if stylelint is not installed but will skip instead.


``openwisp-qa-check``
^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -828,6 +829,8 @@ Shell script to run the following quality assurance checks:
* ``flake8`` - Python code linter
* ``isort`` - Sorts python imports alphabetically, and seperated into sections
* ``black`` - Formats python code using a common standard
* ``csslinter`` - Formats and checks CSS code using stylelint common standard
* ``jslinter`` - Checks Javascript code using jshint common standard

If a check requires a flag, it can be passed forward in the same way.

Expand All @@ -841,6 +844,17 @@ Usage example::

openwisp-qa-check --skip-isort

For backward compatibility ``csslinter`` and ``jslinter`` are skipped by default.
To run them in checks pass arguements in this way.

Usage example::

# To active csslinter
openwisp-qa-check --csslinter

# To activate jslinter
openwisp-qa-check --jslinter

You can do multiple ``checkmigrations`` by passing the arguments with space-delimited string.

For example, this multiple ``checkmigrations``::
Expand Down Expand Up @@ -1118,6 +1132,12 @@ Install test requirements:

pip install -r requirements-test.txt

Install node dependencies used for testing:

.. code-block:: shell

npm install -g stylelint jshint

Set up the pre-push hook to run tests and QA checks automatically right before the git push action, so that if anything fails the push operation will be aborted:

.. code-block:: shell
Expand Down
55 changes: 55 additions & 0 deletions openwisp-qa-check
Expand Up @@ -11,8 +11,11 @@ show_help() {
printf " [--skip-isort]\n"
printf " [--skip-black]\n"
printf " [--skip-checkrst]\n"
printf " [--csslinter]\n"
printf " [--jslinter]\n"
printf "\n"
printf "Runs all checks for quality assurance. Unneeded checks may be skipped by passing --skip-<check-name> argument.\n"
printf "csslinter and jslinter checks are skipped by default for backward compatibility. To run them pass --csslinter or --jslinter as an argument.\n"
printf "\n"
printf "General options:\n"
printf " --help\t\t\t: Show this help text\n"
Expand All @@ -37,6 +40,10 @@ show_help() {
printf " --skip-black\t\t\t: Skip black check\n"
printf "ReStructuredText check options:\n"
printf " --skip-checkrst\t\t: Skip ReStructuredText check\n"
printf "CSSlinter check options:\n"
printf " --csslinter\t\t\t: Runs stylelint check\n"
printf "Jslinter check options:\n"
printf " --jslinter\t\t\t: Runs jshint check\n"
}

echoerr() { echo "$@" 1>&2; }
Expand All @@ -60,6 +67,38 @@ runcheckendline() {
fi
}

runstylelintcheck() {
package='stylelint'
if which stylelint > /dev/null
then
stylelint verbose $(find . -type f -name "*.css") &&
echo "SUCCESS: Stylelint check successful!" ||
{
echoerr "ERROR: Stylelint check failed! Hint: did you forget running openwisp-qa-format? You may need some manual fixing."
FAILURE=1
}
else
echo "MODULE NOT FOUND: Please install ${package}"
FAILURE=1
fi
}

runjshintcheck() {
package='jshint'
if which jshint > /dev/null
then
jshint --verbose $(find . -type f -name "*.js" -a ! -path "*vendor/*.js") &&
echo "SUCCESS: Jshint check successful!" ||
{
echoerr "ERROR: Jshint check failed! Hint: please follow js code conventions. Visit: https://openwisp.io/docs/developer/contributing.html#javascript-code-conventions"
FAILURE=1
}
else
echo "MODULE NOT FOUND: Please install ${package}"
FAILURE=1
fi
}

runcheckmigrations() {
if [ "$MIGRATION_PATH" == "" ]; then
echoerr "SKIPPED: No migration path specified!"
Expand Down Expand Up @@ -151,6 +190,8 @@ SKIP_ISORT=false
SKIP_RSTCHECK=false
SKIP_BLACK=false
SKIP_CHECKCOMMIT=false
SKIP_CSS_LINTER=true
SKIP_JS_LINTER=true
MIGRATION_PATH=""
MIGRATIONS_TO_IGNORE="0"
COMMIT_MESSAGE=$(git log $TRAVIS_PULL_REQUEST_SHA --format=%B -n 1)
Expand Down Expand Up @@ -188,6 +229,12 @@ while [ "$1" != "" ]; do
--skip-checkcommit)
SKIP_CHECKCOMMIT=true
;;
--csslinter)
SKIP_CSS_LINTER=false
;;
--jslinter)
SKIP_JS_LINTER=false
;;
--message)
shift
COMMIT_MESSAGE="$1"
Expand Down Expand Up @@ -235,6 +282,14 @@ if ! $SKIP_FLAKE8; then
runflake8
fi

if ! $SKIP_CSS_LINTER; then
runstylelintcheck
fi

if ! $SKIP_JS_LINTER; then
runjshintcheck
fi

if ! $SKIP_RSTCHECK; then
runrstcheck
fi
Expand Down
7 changes: 7 additions & 0 deletions openwisp-qa-format
@@ -1,5 +1,12 @@
#!/bin/bash
set -e

if which stylelint > /dev/null
then
stylelint $(find . -type f -name "*.css") --fix
else
echo "MODULE NOT FOUND: Please install stylelint"
exit 1
Copy link
Member

Choose a reason for hiding this comment

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

once more time, I think this is wrong. Didn't we agree this feature must be backward compatible?

So why fail if styleling is not installed? This command must not fail if stylelint is missing. It could simply say "The program "stylelint" is not installed, Skipping CSS auto-formatting." or something similar.

fi
isort .
black -S .