Action to enforce python formatting

[2byrds]

Introduce a github action to check python formatting for all PRs being submitted to main, in order to align formatting.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.10%. Comparing base (18d3ad7) to head (1ac2ea6).
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #273      +/-   ##
==========================================
+ Coverage   93.06%   93.10%   +0.04%     
==========================================
  Files          36       36              
  Lines        7121     7221     +100     
==========================================
+ Hits         6627     6723      +96     
- Misses        494      498       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[m00sey]

I think this is solving the wrong problem.

Rather than "reformatting" the entire repo, making future diffs unintelligible, I'd rather see a GitHub action that enforces the current format, rather than switching formats entirely.

[2byrds]

I think this is solving the wrong problem.

Rather than "reformatting" the entire repo, making future diffs unintelligible, I'd rather see a GitHub action that enforces the current format, rather than switching formats entirely.

@m00sey what is the current format(er)?
If the current format(er) is well known then we can apply a check for it. If it is not well known then if we can transition it to a well known format then it is easy for every PR diff to be well formatted, making a diff highlight only content changes (which is ideal)

[2byrds]

I think this is solving the wrong problem.

Rather than "reformatting" the entire repo, making future diffs unintelligible, I'd rather see a GitHub action that enforces the current format, rather than switching formats entirely.

@m00sey I've updated the title, the action only checks the formatting now (enforcing well formatted PRs). It does not try to format it for you any longer. That was discussed on #271 and I had updated it earlier today when I realized that auto-formatting was overkill.

[m00sey]

I think this is solving the wrong problem.
Rather than "reformatting" the entire repo, making future diffs unintelligible, I'd rather see a GitHub action that enforces the current format, rather than switching formats entirely.

@m00sey what is the current format(er)? If the current format(er) is well known then we can apply a check for it. If it is not well known then if we can transition it to a well known format then it is easy for every PR diff to be well formatted, making a diff highlight only content changes (which is ideal)

The current formatting checks are in the CI file:

keria/.github/workflows/python-app-ci.yml

Line 35 in c83904d

# stop the build if there are Python syntax errors or undefined names

Anything proposed down this route should start here.

E9: Syntax Errors - These errors occur when there is an issue with the syntax in the Python code, such as a missing colon, unmatched parentheses, or other fundamental syntax mistakes.
F63: Problem with Import - This code indicates that there is a problem with an import statement, typically an issue with module-level import.
F7: Errors related to function definitions - This includes issues such as missing arguments in function definitions or problems with lambda functions.
F82: Unused local variable - This error occurs when a local variable is defined but not used anywhere in the function or code block.

Ignoring:
E7: Syntax errors related to tokenization - This error indicates issues during the tokenization process, often related to unmatched parentheses or brackets.
F841: Local variable assigned but not used - This occurs when a local variable is assigned a value but is not used in any operations within the code block.
E301: Expected 1 blank line, found 0 - This error is raised when there is not a blank line after a function or class definition.
E302: Expected 2 blank lines, found 1 - This error occurs when there should be two blank lines before a function or class definition, but there is only one.
E303: Too many blank lines (3) - This error occurs when there are more than two consecutive blank lines in the code.

https://flake8.pycqa.org/en/latest/

[2byrds]

I think this is solving the wrong problem.
Rather than "reformatting" the entire repo, making future diffs unintelligible, I'd rather see a GitHub action that enforces the current format, rather than switching formats entirely.

@m00sey what is the current format(er)? If the current format(er) is well known then we can apply a check for it. If it is not well known then if we can transition it to a well known format then it is easy for every PR diff to be well formatted, making a diff highlight only content changes (which is ideal)

The current formatting checks are in the CI file:

keria/.github/workflows/python-app-ci.yml

Line 35 in c83904d

# stop the build if there are Python syntax errors or undefined names

Anything proposed down this route should start here.

E9: Syntax Errors - These errors occur when there is an issue with the syntax in the Python code, such as a missing colon, unmatched parentheses, or other fundamental syntax mistakes. F63: Problem with Import - This code indicates that there is a problem with an import statement, typically an issue with module-level import. F7: Errors related to function definitions - This includes issues such as missing arguments in function definitions or problems with lambda functions. F82: Unused local variable - This error occurs when a local variable is defined but not used anywhere in the function or code block.

Ignoring: E7: Syntax errors related to tokenization - This error indicates issues during the tokenization process, often related to unmatched parentheses or brackets. F841: Local variable assigned but not used - This occurs when a local variable is assigned a value but is not used in any operations within the code block. E301: Expected 1 blank line, found 0 - This error is raised when there is not a blank line after a function or class definition. E302: Expected 2 blank lines, found 1 - This error occurs when there should be two blank lines before a function or class definition, but there is only one. E303: Too many blank lines (3) - This error occurs when there are more than two consecutive blank lines in the code.

https://flake8.pycqa.org/en/latest/

The enabled rules wouldn't help enforce formatting issues for PRs. It is leveraging the syntax linting checks for flake8, and not providing formatting checks. To help distinguish between linters and formatters this is a decent quick read https://py-vscode.readthedocs.io/en/latest/files/linting.html

Note that signify-ts checks formatting and linting:
https://github.com/WebOfTrust/signify-ts/blob/main/.github/workflows/main.yml#L37
https://github.com/WebOfTrust/signify-ts/blob/main/.github/workflows/main.yml#L40

[m00sey]

Okie - just trying to give you a jumping off point.

You managed to create PRs without mass reformatting previously - https://github.com/WebOfTrust/keria/pull/254/files

Maybe you should just go back to that workflow?

Seems like that would expedite the merging process.

[2byrds]

Okie - just trying to give you a jumping off point.

You managed to create PRs without mass reformatting previously - https://github.com/WebOfTrust/keria/pull/254/files

Maybe you should just go back to that workflow?

Seems like that would expedite the merging process.

I'm not sure what you mean. We want to avoid formatting from polluting PRs now and in the future. If we agree on a formatter than that can be easily checked. I have updated this PR to use autopep8 python formatter since @pfeairheller noted he uses PEP in IntelliJ. You can see what the output would look like on the latest formatting check for this PR. I still have no preference of what formatter to use, only that we make formatting simple now and in the future.

[2byrds]

Per our discussion, I'm closing this PR in favor of two separate draft PRs that will allow us to compare an autopep8 formatting check #280 and a Ruff python formatting check #281