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
}
19 changes: 19 additions & 0 deletions .stylelintrc.json
@@ -0,0 +1,19 @@
{
"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"]
}
}
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
62 changes: 62 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,45 @@ runcheckendline() {
fi
}

runstylelintcheck() {
package='stylelint'
if which node > /dev/null
then
if [ `npm list -g | grep -c $package` -eq 0 ]; then
Copy link
Member

Choose a reason for hiding this comment

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

What if I have npm? Does npm come bundled with node?
What do you think about using whereis stylelint instead?
whereis comes out of the box and works great in such situations!

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 will check this and let you know

Copy link
Member Author

Choose a reason for hiding this comment

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

@atb00ker How should I use whereis for this situation?

Copy link
Member

@atb00ker atb00ker Jun 5, 2021

Choose a reason for hiding this comment

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

@niteshsinha17 What do you mean? What's the challenge? Can you please elaborate?
P.S: You can also use which stylelint & which jshint.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes which stylelint will work fine.
I am not getting how to write it with whereis I tried multiple ways but it was not working as expected. Can you please share exact syntax with whereis

Copy link
Member

Choose a reason for hiding this comment

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

Some version of whereis stylelint | grep /stylelint should work but don't worry about it, let's focus on which only, whereis is not a requirement. 😄

echo "MODULE NOT FOUND: please install ${package}"
else
stylelint $(find . -type f -name "*.css") &&
echo "SUCCESS: Stylelint check successful!" ||
{
echoerr "ERROR: Stylelint check failed! Hint: did you forget running openwisp-qa-format?"
FAILURE=1
}
fi
else
echo "MODULE NOT FOUND: please install node and ${package}"
fi
}

runjshintcheck() {
package='jshint'
if which node > /dev/null
then
if [ `npm list -g | grep -c $package` -eq 0 ]; then
echo "MODULE NOT FOUND: please install ${package}"
else
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
}
fi
else
echo "MODULE NOT FOUND: please install node and ${package}"
fi
}

runcheckmigrations() {
if [ "$MIGRATION_PATH" == "" ]; then
echoerr "SKIPPED: No migration path specified!"
Expand Down Expand Up @@ -151,6 +197,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 +236,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 +289,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
1 change: 1 addition & 0 deletions openwisp-qa-format
Expand Up @@ -3,3 +3,4 @@ set -e

isort .
black -S .
stylelint $(find . -type f -name "*.css") --fix
Copy link
Member

Choose a reason for hiding this comment

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

please ensure stylelint is available before trying to run it