priv-kweihmann / multimetric

Calculate code metrics in various languages
zlib License
36 stars 13 forks source link

Begin adding additional documentation #61

Closed cam-barts closed 11 months ago

cam-barts commented 11 months ago

Pull request checklist

Howdy! It's been a bit since we spoke last over email. I've been slowly chipping away at this, and I have a few more changes locally that I'll begin putting in pull requests as soon as I'm confident they are good. I wanted to get the ball rolling on contributions though and give you some peace of mind that I wasn't gonna leave you hanging 😆. This PR should be all documentation adds and some minor formatting things that ruff did in the background. Let me know what you think!

General

Bugfix

New feature

priv-kweihmann commented 11 months ago

Awesome, this is very much appreciated

priv-kweihmann commented 11 months ago

Only the pesky linter is not happy

./multimetric/cls/calc/halstead.py:298:20: C812 missing trailing comma
./multimetric/cls/calc/halstead.py:304:20: C812 missing trailing comma
./multimetric/cls/calc/maintenance.py:230:84: C812 missing trailing comma
./multimetric/cls/calc/maintenance.py:233:86: C812 missing trailing comma
./multimetric/cls/metric/cyclomatic.py:74:89: C812 missing trailing comma
./multimetric/cls/metric/cyclomatic.py:98:51: C812 missing trailing comma
cam-barts commented 11 months ago

@priv-kweihmann excellent, pushed up!

That reminds me, have you considered converting the project from using flake8 to ruff? I can absolutely generate a PR for that conversion, but the main benefits to you are:

I ran it on a branch after this change, with these options: ruff check --ignore PLR2004,S101 . --fix --statistics. PLR2004 is magic value comparison, and S101 is Use of assert detected, but otherwise it just follows my (mostly default) configuration. We could add configuration to the project and it would override. The output of the command is this:

$ ruff check --ignore PLR2004,S101 . --fix --statistics
221 ANN001  Missing type annotation for function argument `_args`
213 ANN101  Missing type annotation for `self` in method
141 ANN201  Missing return type annotation for public function `ArgParser`
126 E501    Line too long (100 > 88 characters)
124 D102    Missing docstring in public method
 83 PTH118  `os.path.join()` should be replaced by `Path` with `/` operator
 74 PLC1901 `captured.err == ""` can be simplified to `not captured.err` as an empty string is falsey
 65 ANN202  Missing return type annotation for private function `__getError`
 57 D100    Missing docstring in public module
 50 D101    Missing docstring in public class
 37 PTH123  `open()` should be replaced by `Path.open()`
 33 N802    Function name `ArgParser` should be lowercase
 24 N803    Argument name `file_BASH` should be lowercase
 18 ANN003  Missing type annotation for `**kwargs`
 17 ARG002  Unused method argument: `args`
 13 D103    Missing docstring in public function
  8 D104    Missing docstring in public package
  8 N806    Variable `RUNARGS` in function should be lowercase
  7 ANN002  Missing type annotation for `*args`
  6 ANN205  Missing return type annotation for staticmethod `_bugpred_new`
  3 BLE001  Do not catch blind exception: `Exception`
  3 ARG001  Unused function argument: `exitstatus`
  2 C901    `get_from_token_tree` is too complex (17 > 7)
  2 D106    Missing docstring in public nested class
  2 D401    First line of docstring should be in imperative mood: "This alters the originally passed metrics by calculated ones."
  2 D404    First word of the docstring should not be "This"
  2 FBT002  Boolean default value in function definition
  2 PTH120  `os.path.dirname()` should be replaced by `Path.parent`
  2 SLF001  Private member accessed: `_effort`
  1 PLR0913 Too many arguments to function call (7 > 5)
  1 PLR0915 Too many statements (65 > 50)
  1 A002    Argument `dir` is shadowing a Python builtin
  1 SIM102  Use a single `if` statement instead of nested `if` statements
  1 SIM108  Use ternary operator `item = item.strip(i) if len(i) == 1 else item.replace(i, "")` instead of `if`-`else`-block
  1 S113    Probable use of requests call without timeout
  1 FBT001  Boolean positional arg in function definition
  1 INP001  File `tests/__data_local/test.py` is part of an implicit namespace package. Add an `__init__.py`.
  1 TRY002  Create your own exception
  1 TRY300  Consider moving this statement to an `else` block
  1 PTH100  `os.path.abspath()` should be replaced by `Path.resolve()`
  1 PTH103  `os.makedirs()` should be replaced by `Path.mkdir(parents=True)`
  1 PTH110  `os.path.exists()` should be replaced by `Path.exists()`
  1 PTH122  `os.path.splitext()` should be replaced by `Path.suffix`

When you diff the changes, it's mostly changing single quotes to double quotes, adding type hints, and reorganizing imports. I can put up a WIP PR for more discussion if you like, and we can piece through the tools output, but I wanted to make sure it was a direction you were comfortable with.

priv-kweihmann commented 11 months ago

@priv-kweihmann excellent, pushed up!

That reminds me, have you considered converting the project from using flake8 to ruff? I can absolutely generate a PR for that conversion, but the main benefits to you are:

  • Ruff is significantly faster than flake8
  • Ruff checks for all of the flake8 rules, plus some very sensible additional ones
  • Ruff can be added to CI in the same way that flake8 can
  • Ruff has a feature to fix some common issues (like the trailing comma, quote issues, and even a few type hints) on it's own

I ran it on a branch after this change, with these options: ruff check --ignore PLR2004,S101 . --fix --statistics. PLR2004 is magic value comparison, and S101 is Use of assert detected, but otherwise it just follows my (mostly default) configuration. We could add configuration to the project and it would override. The output of the command is this:

$ ruff check --ignore PLR2004,S101 . --fix --statistics
221   ANN001  Missing type annotation for function argument `_args`
213   ANN101  Missing type annotation for `self` in method
141   ANN201  Missing return type annotation for public function `ArgParser`
126   E501    Line too long (100 > 88 characters)
124   D102    Missing docstring in public method
 83   PTH118  `os.path.join()` should be replaced by `Path` with `/` operator
 74   PLC1901 `captured.err == ""` can be simplified to `not captured.err` as an empty string is falsey
 65   ANN202  Missing return type annotation for private function `__getError`
 57   D100    Missing docstring in public module
 50   D101    Missing docstring in public class
 37   PTH123  `open()` should be replaced by `Path.open()`
 33   N802    Function name `ArgParser` should be lowercase
 24   N803    Argument name `file_BASH` should be lowercase
 18   ANN003  Missing type annotation for `**kwargs`
 17   ARG002  Unused method argument: `args`
 13   D103    Missing docstring in public function
  8   D104    Missing docstring in public package
  8   N806    Variable `RUNARGS` in function should be lowercase
  7   ANN002  Missing type annotation for `*args`
  6   ANN205  Missing return type annotation for staticmethod `_bugpred_new`
  3   BLE001  Do not catch blind exception: `Exception`
  3   ARG001  Unused function argument: `exitstatus`
  2   C901    `get_from_token_tree` is too complex (17 > 7)
  2   D106    Missing docstring in public nested class
  2   D401    First line of docstring should be in imperative mood: "This alters the originally passed metrics by calculated ones."
  2   D404    First word of the docstring should not be "This"
  2   FBT002  Boolean default value in function definition
  2   PTH120  `os.path.dirname()` should be replaced by `Path.parent`
  2   SLF001  Private member accessed: `_effort`
  1   PLR0913 Too many arguments to function call (7 > 5)
  1   PLR0915 Too many statements (65 > 50)
  1   A002    Argument `dir` is shadowing a Python builtin
  1   SIM102  Use a single `if` statement instead of nested `if` statements
  1   SIM108  Use ternary operator `item = item.strip(i) if len(i) == 1 else item.replace(i, "")` instead of `if`-`else`-block
  1   S113    Probable use of requests call without timeout
  1   FBT001  Boolean positional arg in function definition
  1   INP001  File `tests/__data_local/test.py` is part of an implicit namespace package. Add an `__init__.py`.
  1   TRY002  Create your own exception
  1   TRY300  Consider moving this statement to an `else` block
  1   PTH100  `os.path.abspath()` should be replaced by `Path.resolve()`
  1   PTH103  `os.makedirs()` should be replaced by `Path.mkdir(parents=True)`
  1   PTH110  `os.path.exists()` should be replaced by `Path.exists()`
  1   PTH122  `os.path.splitext()` should be replaced by `Path.suffix`

When you diff the changes, it's mostly changing single quotes to double quotes, adding type hints, and reorganizing imports. I can put up a WIP PR for more discussion if you like, and we can piece through the tools output, but I wanted to make sure it was a direction you were comfortable with.

I've seen ruff and it looks interesting, but as the project itself claims "it's highly actively developed" and I like my tools a bit more stable, if not to say at best "debian" kind of stable. So ruff might be something for the long run and if I would roll it out to all of my projects (mainly using the same toolset for almost everything, which makes maintaining them much easier).

I will take a deeper look what tools can be replaced by ruff if my time allows it, so could you please create an issue as a reminder for my please 😃