neuroevolution-ai / NeuroEvolution-CTRNN_new

MIT License
3 stars 3 forks source link

style guide #75

Open bjuergens opened 3 years ago

bjuergens commented 3 years ago

I noticed in some PR there is a lot of clutter because of inconsistent style.

We should define some style-guides (e.g. " vs ' for strings, for order of imports, and for formatting string). We should just follow the official style guides as far as possible.


Here is an example how bad formatting can create problems:

image

at first glance it looks, like nothing has changed at all. You have to consciously look for differences to notice, that the import of ConfigClass has been removed.

I suggest we simply use the "Optimize Imports" feature of pycharm; https://www.jetbrains.com/help/pycharm/creating-and-optimizing-imports.html#optimize-imports-in-project

There is a official python style guide on this, which -convieniently- describes exactly what the pycharm feature does already: https://www.python.org/dev/peps/pep-0008/#imports


Adopting this change would come with the follow practical changes:

bjuergens commented 3 years ago

some info for github actions for style checks:

basically this can be implemented by adding 6 lines to the CI-config:

- name: wemake-python-styleguide
  uses: wemake-services/wemake-python-styleguide
  with:
    reporter: 'github-pr-check'
  env:
    GITHUB_TOKEN: ${{ secrets.github_token }}

I am a little confused/annoyed, that a secret token needs to be passed to it though.

note: This tool can be used as a simple checker, that adds a little yellow flag at the end of a PR, when the style check fails. But It can also act as a reviewer, that provides a code-review and automatically suggestests changes.

pdeubel commented 3 years ago

I don't like the secret token either, maybe it is necessary so that it can comment on files to do its code-review.

Style related: I use Optimize Imports and Rearrange Code from PyCharm and double quotes for all stings. Other than that I try to follow Pep8.

As long as this is done with any tool in the Action Runner I'm fine with it. Did you do #82 by hand or can this be done in the Action Runner automatically?

bjuergens commented 3 years ago

Did you do #82 by hand or can this be done in the Action Runner automatically?

pycharm somewhere has a button that runs the code-formatter on all files in a project. That's what I used for #82.

pdeubel commented 3 years ago

Ok I would vote for pycharm