Add pre-commit for better linting #25

bmchalenv · 2026-01-14T07:15:39Z

Adds a pre-commit config that uses uncrustify and flake8 just like ament. ament isn't used directly since I wanted formatting to occur outside of a ROS environment when making small fixes. Also has other tools like whitespace, newline, copyright, yaml checks, code spelling, and shell script formatting.

Useful for fixing most issues with linting before it is pushed. Automatically reformats your commits, fixes copyright year issues, etc.

To run:

sudo apt install uncrustify pipx
pipx install pre-commit
pre-commit run --all-files  # will also run on every commit now

greptile-apps · 2026-01-14T07:18:44Z

Greptile Overview

Greptile Summary

This PR adds a comprehensive pre-commit configuration that automates code quality checks, formatting, and validation across the codebase. The configuration includes Python formatting (autopep8/flake8), C++ formatting (uncrustify), shell script formatting (shfmt), copyright checks, YAML validation, and sign-off verification.

Major changes:

Added .pre-commit-config.yaml with 9 hooks for linting and formatting
Integrated pre-commit checks into CI workflow (.github/workflows/ros-tests.yml)
Updated Contributing.md with pre-commit installation instructions
Applied automated formatting to all files (copyright years updated to 2025-2026, whitespace fixes, shell scripts reformatted with tabs)

Issues identified:

CI workflow is missing uncrustify installation, which will cause pre-commit to fail in GitHub Actions
Minor grammar issue in Contributing.md documentation

Previous issues resolved:

Fixed: package.xml no longer has duplicate XML declarations
Fixed: Python formatting issues from aggressive autopep8 have been corrected (changed from -a -a to -i)
Fixed: Pre-commit now runs in correct working directory

Confidence Score: 3/5

Mostly safe to merge, but CI will fail without uncrustify installation fix
The pre-commit configuration itself is well-structured and addresses code quality concerns. Most formatting issues from previous review rounds have been fixed. However, the missing uncrustify dependency in the CI workflow will cause GitHub Actions to fail, making this a blocking issue that must be resolved before merge.
.github/workflows/ros-tests.yml requires uncrustify to be added to apt install command on line 60

Important Files Changed

Filename	Overview
.pre-commit-config.yaml	Adds comprehensive pre-commit configuration with linting, formatting, and validation hooks
.github/workflows/ros-tests.yml	Integrates pre-commit checks into CI workflow, missing `uncrustify` installation
Contributing.md	Adds pre-commit setup instructions with grammar issue in last sentence

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Git as Git Commit
    participant PreCommit as Pre-commit Hooks
    participant CI as GitHub Actions
    participant Build as Build System

    Dev->>Git: git commit -s -m "message"
    Git->>PreCommit: Trigger pre-commit hooks
    
    PreCommit->>PreCommit: Run check-yaml
    PreCommit->>PreCommit: Run end-of-file-fixer
    PreCommit->>PreCommit: Run trailing-whitespace
    PreCommit->>PreCommit: Run autopep8 (Python formatting)
    PreCommit->>PreCommit: Run flake8 (Python linting)
    PreCommit->>PreCommit: Run shfmt (Shell formatting)
    PreCommit->>PreCommit: Run uncrustify (C++ formatting)
    PreCommit->>PreCommit: Run copyright-required
    PreCommit->>PreCommit: Run codespell
    PreCommit->>PreCommit: Check signed-off-by in commit msg
    
    alt Hooks Pass
        PreCommit-->>Git: Success
        Git-->>Dev: Commit created
        Dev->>CI: Push to GitHub
        CI->>PreCommit: Run pre-commit --all-files
        CI->>Build: Build and test packages
        Build-->>CI: Tests pass
    else Hooks Fail
        PreCommit-->>Git: Failure (auto-fix applied)
        Git-->>Dev: Changes staged, retry commit
    end

greptile-apps

_{23 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greenwave_monitor_interfaces/package.xml

greptile-apps

_{23 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greenwave_monitor_interfaces/package.xml

greptile-apps

_{23 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-14T16:53:43Z

greenwave_monitor/greenwave_monitor/ncurses_frontend.py

+        should_redraw = (
+            key != -
+            1) or (
+            current_time -
+            last_redraw >= redraw_interval)


syntax: Autopep8 broke this expression by splitting -1 across lines. key != - on one line and 1) on the next is incorrect formatting.

Suggested change

should_redraw = (

key != -

1) or (

current_time -

last_redraw >= redraw_interval)

should_redraw = (key != -1) or (current_time - last_redraw >= redraw_interval)

greptile-apps · 2026-01-14T16:53:44Z

greenwave_monitor/greenwave_monitor/ncurses_frontend.py

+        header = (
+            f'{
+                "Topic Name":<{MAX_NAME_WIDTH}} {
+                "Status":<{STATUS_WIDTH}} ' f'{
+                "Pub Rate (Hz)":<{FRAME_RATE_WIDTH}} ' f'{
+                    "Latency (ms)":<{REALTIME_DELAY_WIDTH}} {
+                        "Expected Hz":<12}')


syntax: F-string formatting is broken. Line breaks inside the f-string placeholders create invalid syntax.

Suggested change

header = (

f'{

"Topic Name":<{MAX_NAME_WIDTH}} {

"Status":<{STATUS_WIDTH}} ' f'{

"Pub Rate (Hz)":<{FRAME_RATE_WIDTH}} ' f'{

"Latency (ms)":<{REALTIME_DELAY_WIDTH}} {

"Expected Hz":<12}')

header = (f'{"Topic Name":<{MAX_NAME_WIDTH}} {"Status":<{STATUS_WIDTH}} '

f'{"Pub Rate (Hz)":<{FRAME_RATE_WIDTH}} '

f'{"Latency (ms)":<{REALTIME_DELAY_WIDTH}} {"Expected Hz":<12}')

greptile-apps · 2026-01-14T16:53:45Z

greenwave_monitor/greenwave_monitor/ncurses_frontend.py

+            topic_set = {
+                topic_name for topic_name,
+                topic_type in topic_names_and_types}


syntax: Set comprehension incorrectly split. The comma between topic_name and topic_type should be on the same line.

Suggested change

topic_set = {

topic_name for topic_name,

topic_type in topic_names_and_types}

topic_set = {topic_name for topic_name, topic_type in topic_names_and_types}

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

greptile-apps · 2026-01-14T16:53:46Z

.pre-commit-config.yaml

+    - id: autopep8
+      args: [-i, -a, -a]


style: The -a -a flags make autopep8 very aggressive, causing the broken formatting in ncurses_frontend.py. Consider using just -i or single -a to avoid overly aggressive line breaking.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{3 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T01:01:14Z

.github/workflows/ros-tests.yml

+    - name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)
+      run: |
+        apt update && apt install -y pipx git
+        pipx install pre-commit
+        pipx ensurepath
+        source ~/.bashrc
+        pre-commit run --all-files --show-diff-on-failure
+      shell: bash
+      working-directory: greenwave_monitor
+      env:


Pre-commit runs in wrong directory. Code is checked out to greenwave_monitor/ subdirectory (line 56) but this step doesn't cd into it before running pre-commit.

Suggested change

- name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)

run: |

apt update && apt install -y pipx git

pipx install pre-commit

pipx ensurepath

source ~/.bashrc

pre-commit run --all-files --show-diff-on-failure

shell: bash

working-directory: greenwave_monitor

env:

- name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)

run: |

cd greenwave_monitor

apt update && apt install -y pipx git

pipx install pre-commit

pipx ensurepath

source ~/.bashrc

pre-commit run --all-files --show-diff-on-failure

shell: bash

env:

DEBIAN_FRONTEND: noninteractive

greptile-apps · 2026-01-28T01:01:15Z

Contributing.md

+pre-commit run --all-files  # try it out, this will run every commit now
+```
+
+Every commit now a series of checks will be run to ensure the changes are meeting this repositories requirements.


Grammar: missing verb. Should be "Every commit now, a series" or "On every commit now a series"

Suggested change

Every commit now a series of checks will be run to ensure the changes are meeting this repositories requirements.

On every commit now a series of checks will be run to ensure the changes are meeting this repositories requirements.

greptile-apps · 2026-01-28T01:01:16Z

.github/workflows/ros-tests.yml


+    - name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)
+      run: |
+        apt update && apt install -y pipx git


uncrustify dependency missing. Pre-commit config uses uncrustify (line 46 of .pre-commit-config.yaml) but it's not installed here. Contributing.md also mentions it's required.

Suggested change

apt update && apt install -y pipx git

apt update && apt install -y pipx git uncrustify

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T01:09:18Z

.github/workflows/ros-tests.yml


+    - name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)
+      run: |
+        apt update && apt install -y pipx git


uncrustify is required by the pre-commit config (line 46 of .pre-commit-config.yaml) but isn't installed here. Add it to the apt install command.

Suggested change

apt update && apt install -y pipx git

apt update && apt install -y pipx git uncrustify

greptile-apps · 2026-01-28T01:09:19Z

Contributing.md

+pre-commit run --all-files  # try it out, this will run every commit now
+```
+
+Every commit now a series of checks will be run to ensure the changes are meeting this repositories requirements.


Grammar issue: missing verb or incorrect structure. Should be "On every commit now, a series" or "Every commit now runs a series"

Suggested change

Every commit now a series of checks will be run to ensure the changes are meeting this repositories requirements.

On every commit now a series of checks will be run to ensure the changes are meeting this repositories requirements.

greptile-apps bot reviewed Jan 14, 2026

View reviewed changes

greenwave_monitor_interfaces/package.xml Show resolved Hide resolved

greptile-apps bot reviewed Jan 14, 2026

View reviewed changes

greenwave_monitor_interfaces/package.xml Show resolved Hide resolved

greptile-apps bot reviewed Jan 14, 2026

View reviewed changes

bmchalenv added 7 commits January 27, 2026 13:52

Add pre-commit for linting locally

300f848

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Beginning of linting journey

5ba1541

Signed-off-by: Blake McHale <bmchale@nvidia.com>

More config fixes

dfdf823

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Match flake8 to ament_flake8

624776f

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Run lint again

e42b1d6

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Only support python 3.10 features

1fa80bf

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Only support python 3.10 features

900a432

Signed-off-by: Blake McHale <bmchale@nvidia.com>

bmchalenv force-pushed the pre-commit-init branch from 8b43cc2 to ac97d27 Compare January 27, 2026 23:39

greptile-apps bot reviewed Jan 27, 2026

View reviewed changes

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

bmchalenv added 8 commits January 27, 2026 16:36

Fix line length issue

5c9e1cc

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Reset to main

5898ef0

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Fix lints

0489236

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Update Contributing.md to reference installation steps

1ff3421

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Add lint tests as first step since it should fail fast

eb66360

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Tests lint in CI

da401d0

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Check sign off pre-commit

b09b7b9

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Check commit

a016e5c

Signed-off-by: Blake McHale <bmchale@nvidia.com>

bmchalenv force-pushed the pre-commit-init branch from 993b9ef to a016e5c Compare January 28, 2026 00:52

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

bmchalenv added 2 commits January 27, 2026 16:57

Update md

d676849

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Use proper working directory

e819d07

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

2f4721d

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

c85bec4

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

	Every commit now a series of checks will be run to ensure the changes are meeting this repositories requirements.
	On every commit now a series of checks will be run to ensure the changes are meeting this repositories requirements.

	apt update && apt install -y pipx git
	apt update && apt install -y pipx git uncrustify

Add pre-commit for better linting #25

Are you sure you want to change the base?

Add pre-commit for better linting #25

Conversation

bmchalenv commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

bmchalenv commented Jan 14, 2026 •

edited

Loading

greptile-apps bot commented Jan 14, 2026 •

edited

Loading