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

Add pylint to CI and fix all Python #3052

Merged
merged 1 commit into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,7 @@ function_next_line = false

[libs/**]
ignore = true

[*.py]
# This is the widest that is still PEP-8 compliant
max_line_length = 99
38 changes: 20 additions & 18 deletions .github/autobuild/get_build_vars.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#!/usr/bin/env python3
# This script is trigged from the Github Autobuild workflow.
# It analyzes Jamulus.pro and git push details (tag vs. branch, etc.) to decide
# - whether a release should be created,
# - whether it is a pre-release, and
# - what its title should be.
"""
This script is triggered from the GitHub Autobuild workflow.
It analyzes Jamulus.pro and git push details (tag vs. branch, etc.) to decide
- whether a release should be created,
- whether it is a pre-release, and
- what its title should be.
"""

import os
import re
Expand All @@ -15,10 +17,10 @@
def get_version_from_jamulus_pro():
ann0see marked this conversation as resolved.
Show resolved Hide resolved
with open(REPO_PATH + '/Jamulus.pro', 'r') as f:
pro_content = f.read()
m = re.search(r'^VERSION\s*=\s*(\S+)$', pro_content, re.MULTILINE)
if not m:
raise Exception("Unable to determine Jamulus.pro VERSION")
return m.group(1)
matches = re.search(r'^VERSION\s*=\s*(\S+)$', pro_content, re.MULTILINE)
if not matches:
raise ValueError("Unable to determine Jamulus.pro VERSION")
return matches.group(1)


def get_git_hash():
Expand All @@ -34,26 +36,26 @@ def get_git_hash():

def get_build_version(jamulus_pro_version):
if "dev" in jamulus_pro_version:
version = "{}-{}".format(jamulus_pro_version, get_git_hash())
version = f"{jamulus_pro_version}-{get_git_hash()}"
return 'intermediate', version

version = jamulus_pro_version
return 'release', version


def set_github_variable(varname, varval):
print("{}='{}'".format(varname, varval)) # console output
outputfile = os.getenv('GITHUB_OUTPUT')
with open(outputfile, "a") as ghout:
print(f"{varname}='{varval}'") # console output
output_file = os.getenv('GITHUB_OUTPUT')
with open(output_file, "a") as ghout:
ghout.write(f"{varname}={varval}\n")

jamulus_pro_version = get_version_from_jamulus_pro()
set_github_variable("JAMULUS_PRO_VERSION", jamulus_pro_version)
build_type, build_version = get_build_version(jamulus_pro_version)
print(f'building a version of type "{build_type}": {build_version}')

fullref = os.environ['GITHUB_REF']
publish_to_release = bool(re.match(r'^refs/tags/r\d+_\d+_\d+\S*$', fullref))
full_ref = os.environ['GITHUB_REF']
publish_to_release = bool(re.match(r'^refs/tags/r\d+_\d+_\d+\S*$', full_ref))

# BUILD_VERSION is required for all builds including branch pushes
# and PRs:
Expand All @@ -65,12 +67,12 @@ def set_github_variable(varname, varval):
set_github_variable("PUBLISH_TO_RELEASE", str(publish_to_release).lower())

if publish_to_release:
reflist = fullref.split("/", 2)
release_tag = reflist[2]
ref_list = full_ref.split("/", 2)
release_tag = ref_list[2]
release_title = f"Release {build_version} ({release_tag})"
is_prerelease = not re.match(r'^r\d+_\d+_\d+$', release_tag)
if not is_prerelease and build_version != release_tag[1:].replace('_', '.'):
raise Exception(f"non-pre-release tag {release_tag} doesn't match Jamulus.pro VERSION = {build_version}")
raise ValueError(f"non-pre-release tag {release_tag} doesn't match Jamulus.pro VERSION = {build_version}")

# Those variables are only used when a release is created at all:
set_github_variable("IS_PRERELEASE", str(is_prerelease).lower())
Expand Down
22 changes: 20 additions & 2 deletions .github/workflows/coding-style-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,22 @@ on:
- '**.h'
- '**.mm'
- '**.sh'
- '**.py'
pull_request:
branches: [ main ]
paths:
- '**.cpp'
- '**.h'
- '**.mm'
- '**.sh'
- '**.py'

permissions:
contents: read

jobs:
check-coding-style:
name: Verify coding style
check-c-like-style:
name: Verify C-like coding style
# As this action runs with minimal permissions and does not influence the build output,
# we perform no version pinning for the runner or the installed tools here.
# The clangFormatVersion is based on Ubuntu current LTS (jammy at time of writing).
Expand All @@ -36,10 +38,26 @@ jobs:
# When updating the extension list, remember to update
# Jamulus.pro's CLANG_SOURCES as well and the paths: in 'on:' list above.
extensions: 'cpp,h,mm'

check-bash-style:
name: Verify shell script (Bash) coding style
runs-on: ubuntu-latest
# shellcheck is already pre-installed on ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Check .sh with shellcheck
run: find -name '*.sh' -not -path './libs/*' -exec shellcheck --shell=bash {} +
- name: Install shfmt
run: sudo snap install shfmt
- name: Check .sh with shfmt
run: shfmt -d .

check-python-style:
name: Verify Python coding style
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Install pylint
run: pip install --user "pylint < 3.0"
ann0see marked this conversation as resolved.
Show resolved Hide resolved
- name: Check Python files with pylint
run: find ./tools -name '*.py' -print -exec pylint {} +
39 changes: 39 additions & 0 deletions .pylintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
[MAIN]

# Specify a score threshold under which the program will exit with error.
fail-under=10
ann0see marked this conversation as resolved.
Show resolved Hide resolved

[BASIC]

# Minimum line length for functions/classes that require docstrings, shorter
# ones are exempt.
docstring-min-length=100
ann0see marked this conversation as resolved.
Show resolved Hide resolved


# Good variable names which should always be accepted, separated by a comma.
good-names=s, f, i, j, k, e, _

# Include a hint for the correct naming format with invalid-name.
include-naming-hint=yes

[FORMAT]

# Maximum number of characters on a single line.
# See .editorconfig
max-line-length=99

disable=
# Disable enforcing constants as ALL_CAPS -
# pylint produces too many false positives here
C0103,
# Allow redefining names from outer scope
W0621,
# Assume utf-8 everywhere
W1514,
ann0see marked this conversation as resolved.
Show resolved Hide resolved
# Allow classes with any number of public methods
R0903,
ann0see marked this conversation as resolved.
Show resolved Hide resolved
# Allow TODOs in code
W0511

output-format=colorized

9 changes: 8 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ If a feature or function can be accomplished in another way by another system or

### Source code consistency

#### C-like languages
Please install and run `clang-format` on your PC **before committing** to maintain a consistent coding style. You should use the version we use to validate the style in our CI [(see our coding-style-check file and look for the `clangFormatVersion`)](https://github.com/jamulussoftware/jamulus/blob/main/.github/workflows/coding-style-check.yml#L20). Our CI will fail and tell you about styling violations.

There are several ways to run clang-format:
Expand All @@ -30,7 +31,7 @@ There are several ways to run clang-format:

- By hand: run `clang-format -i <path/to/changed/files>`

#### Style definition
##### Style definition

Please see the [.clang_format file](https://github.com/jamulussoftware/jamulus/blob/main/.clang-format) in the root folder. In summary:

Expand All @@ -40,6 +41,12 @@ Please see the [.clang_format file](https://github.com/jamulussoftware/jamulus/b
- Enclose all bodies of `if`, `else`, `while`, `for`, etc. in braces `{` and `}` on separate lines.
- Do not use concatinations in strings with parameters. Instead use substitutions. **Do:** `QString ( tr ( "Hello, %1. Have a nice day!" ) ).arg( getName() )` **Don't:** `tr ( "Hello " ) + getName() + tr ( ". Have a nice day!" )` ...to make translation easier.

#### Python
Please install and use [pylint](https://pylint.org/) to scan any Python code.
There is a configuration file that defines some overrides,
and note the [Editorconfig file](.editorconfig) in the project too.


### Supported platforms

We support the following platforms and versions:
Expand Down
2 changes: 1 addition & 1 deletion Jamulus.pro
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,7 @@ DISTFILES += ChangeLog \
tools/create-translation-issues.sh \
tools/generate_json_rpc_docs.py \
tools/get_release_contributors.py \
tools/qt5-to-qt6-country-code-table.py \
tools/qt5_to_qt6_country_code_table.py \
tools/update-copyright-notices.sh \
windows/deploy_windows.ps1 \
windows/installer.nsi
Expand Down
2 changes: 1 addition & 1 deletion src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,7 @@ class CLocale
static bool IsCountryCodeSupported ( unsigned short iCountryCode );
static QLocale::Country GetCountryCodeByTwoLetterCode ( QString sTwoLetterCode );
#if QT_VERSION >= QT_VERSION_CHECK( 6, 0, 0 )
// ./tools/qt5-to-qt6-country-code-table.py generates these lists:
// ./tools/qt5_to_qt6_country_code_table.py generates these lists:
constexpr int const static wireFormatToQt6Table[] = {
0, 1, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28,
29, 30, 31, 32, 33, 35, 36, 37, 38, 39, 40, 41, 43, 45, 46, 48, 49, 50, 51, 53, 54, 55, 57, 56, 58, 59, 118,
Expand Down
21 changes: 8 additions & 13 deletions tools/generate_json_rpc_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ class DocumentationItem:
def __init__(self, name, type_):
"""
@param name: str
@param type: str
@param type_: str
ann0see marked this conversation as resolved.
Show resolved Hide resolved
"""
self.name = name
self.type = type_
self.brief = DocumentationText()
self.params = []
self.results = []
self.current_tag = None
ann0see marked this conversation as resolved.
Show resolved Hide resolved

def handle_tag(self, tag_name):
"""
Expand All @@ -57,7 +58,7 @@ def handle_tag(self, tag_name):
self.current_tag = DocumentationText()
self.results.append(self.current_tag)
else:
raise Exception("Unknown tag: " + tag_name)
raise ValueError("Unknown tag: " + tag_name)

def handle_text(self, text):
"""
Expand All @@ -75,11 +76,7 @@ def to_markdown(self):
"""
@return: Markdown-formatted str with name, brief, params and results
"""
output = []
output.append("### " + self.name)
output.append("")
output.append(str(self.brief))
output.append("")
output = ["### " + self.name, "", str(self.brief), ""]
ann0see marked this conversation as resolved.
Show resolved Hide resolved

if len(self.params) > 0:
output.append("Parameters:")
Expand Down Expand Up @@ -133,10 +130,8 @@ def to_markdown(self):
"""
@return: str containing the Markdown table
"""
output = []
output.append("| Name | Type | Description |")
output.append("| --- | --- | --- |")
tag_re = re.compile(r"^\{(\w+)\}\s+([\S]+)\s+-\s+(.*)$", re.DOTALL)
output = ["| Name | Type | Description |", "| --- | --- | --- |"]
tag_re = re.compile(r"^{(\w+)}\s+(\S+)\s+-\s+(.*)$", re.DOTALL)
ann0see marked this conversation as resolved.
Show resolved Hide resolved
for tag in self.tags:
text = str(tag)
# Parse tag in form of "{type} name - description"
Expand Down Expand Up @@ -185,7 +180,7 @@ def to_markdown(self):

items.sort(key=lambda item: item.name)

preamble = """
PREAMBLE = """
# Jamulus JSON-RPC Documentation

<!--
Expand Down Expand Up @@ -251,7 +246,7 @@ def to_markdown(self):

docs_path = os.path.join(repo_root, 'docs', 'JSON-RPC.md')
with open(docs_path, "w") as f:
f.write(preamble)
f.write(PREAMBLE)

f.write("## Method reference\n")
for item in filter(lambda item: item.type == "method", items):
Expand Down
Loading
Loading