Apply black formatter and add github action to check conformity

[heuristicus]

As mentioned in #56, I think having a standard formatting scheme would help with consistency in the code here. There is more work to be done beyond this simple formatting application, but this would be a good start.

Summary

I have applied the black code formatter to the src and scripts directories of the driver.

You can find the documentation for the formatter at https://black.readthedocs.io/en/latest/index.html. It is used by numerous large python projects to apply consistent formatting. Since there are so few options, it means that there are not many things to argue about in terms of the details of formatting.

The formatter guarantees that the formatted code is exactly the same as the input code by comparing the AST before and after the formatting. If the AST would be changed by the formatting, the formatting fails.

Installation and usage

Installation on ubuntu is just

pip3 install black

You can then run on a directory or specific file with

black [directory-or-file]

In this repository, the command would be

black spot_driver/src spot_driver/scripts

There are instructions at https://black.readthedocs.io/en/latest/integrations/editors.html to add the formatter to your editor, making it very simple to run.

Github action

In addition to applying formatting here, I have added a github action which will trigger on pushes and pull requests, and give a :x: or :heavy_check_mark: next to the PR or commit, indicating whether the code would be changed by running the formatter. This allows us to ensure that the formatting scheme is maintained by PRs.

Git pre-commit hook

You can also make it so that git will reject attempts to make a commit where the code would be changed by applying the formatter. To do this, you can use a pre-commit hook. In all git repositories on your machine you will have a directory .git/hooks, which by default contains a lot of sample files.

The minimal hook that you need to reject commits is

#!/bin/sh

black --check spot_driver/src spot_driver/scripts

if [ $? != 0 ]; then
        echo "Pre-commit black check failed. Run 'black spot_driver/src spot_driver/scripts' to fix"
        exit 1
fi

You can use this short script to install this into your repo (this will overwrite any existing pre-commit hook you have)

roscd spot_driver/..
cat > .git/hooks/pre-commit<<EOF
 #!/bin/sh

black --check spot_driver/src spot_driver/scripts

if [ \$? != 0 ]; then
        echo "Pre-commit black check failed. Run 'black spot_driver/src spot_driver/scripts' to fix"
        exit 1
fi
EOF
chmod +x .git/hooks/pre-commit

With that script in place, whenever you commit you will see

Formatting is correct, the commit goes through as normal after the check

michal@vetinari ~/robots/drs/spot_ws/src/spot_ros $ git status
On branch improvement/black-formatting
Your branch is up-to-date with 'origin/improvement/black-formatting'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   spot_driver/src/spot_driver/spot_ros.py

michal@vetinari ~/robots/drs/spot_ws/src/spot_ros $ git commit -m 'test'
All done! ✨ 🍰 ✨
5 files would be left unchanged.
[improvement/black-formatting da74556] test
 1 file changed, 1 insertion(+)
michal@vetinari ~/robots/drs/spot_ws/src/spot_ros $ git status
On branch improvement/black-formatting
Your branch is ahead of 'origin/improvement/black-formatting' by 1 commit.
  (use "git push" to publish your local commits)

nothing to commit, working tree clean

Formatting is not correct, the commit will not go through

michal@vetinari ~/robots/drs/spot_ws/src/spot_ros $ git status
On branch improvement/black-formatting
Your branch is up-to-date with 'origin/improvement/black-formatting'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   spot_driver/src/spot_driver/spot_ros.py

michal@vetinari ~/robots/drs/spot_ws/src/spot_ros $ git commit -m 'test'
would reformat spot_driver/src/spot_driver/spot_ros.py
Oh no! 💥 💔 💥
1 file would be reformatted, 4 files would be left unchanged.
Pre-commit black check failed. Run 'black spot_driver/src spot_driver/scripts' to fix
michal@vetinari ~/robots/drs/spot_ws/src/spot_ros $ git status
On branch improvement/black-formatting
Your branch is up-to-date with 'origin/improvement/black-formatting'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   spot_driver/src/spot_driver/spot_ros.py

[civerachb-cpr]

I appreciate the enthusiasm, but I'm hesitant to introduce non-standard tools, even if they are widely-used. The ROS linter is already capable of handling the most egregious of formatting problems.

Looking at the changes you made, besides adding the formatter itself, most of the changes seem to be consistently using double-quotes instead of single-quotes for strings, and reducing the maximum line width from the 120 characters the ROS linter is already using.

[heuristicus]

I have to admit that I'm not familiar with roslint. Looking into it a little, adding roslint_python to a package's CMakeLists just provides a new catkin command that can be run. In the case of this repository it would require running something like catkin roslint_spot_driver. This command applies pycodestyle to the listed files, which does not actually apply any formatting, but just outputs information about what does not conform to the python code style. That means that it's up to you to manually change each of these lines.

The key with the change I'm suggesting here is that the black formatter will automatically apply changes which conform to most of the PEP8 formatting guidelines (except line length, although that can be configured if desired). This takes manual effort out of the formatting process.