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

Create reusable-pre-commit.yml and add workflow for it #10

Merged
merged 19 commits into from
Feb 21, 2024
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
11 changes: 11 additions & 0 deletions .github/workflows/ci-pre-commit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: Format

on:
workflow_dispatch:
pull_request:

jobs:
pre-commit:
uses: ./.github/workflows/reusable-pre-commit.yml
with:
ros_distro: rolling
33 changes: 33 additions & 0 deletions .github/workflows/reusable-pre-commit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
name: Pre-commit
# The pre-commit configuration is in .pre-commit-config.yaml
# we don't use the pre-commit action because it doesn't support local hooks in its virtual environment

on:
workflow_call:
inputs:
ros_distro:
description: 'ROS2 distribution name'
required: true
type: string
os_name:
description: 'On which OS to run the linter'
required: false
default: 'ubuntu-latest'
type: string

jobs:
pre-commit:
runs-on: ${{ inputs.os_name }}
steps:
- uses: actions/checkout@v4
- uses: ros-tooling/setup-ros@0.7.1
- uses: actions/cache@v4
with:
path: ~/.cache/pre-commit
key: pre-commit-3|${{ inputs.ros_distro }}|${{ hashFiles('.pre-commit-config.yaml') }}
- name: Install system hooks and run pre-commit
run: |
sudo apt-get install -qq ros-${{ inputs.ros_distro }}-ament-cppcheck ros-${{ inputs.ros_distro }}-ament-cpplint ros-${{ inputs.ros_distro }}-ament-lint-cmake ros-${{ inputs.ros_distro }}-ament-copyright
source /opt/ros/${{ inputs.ros_distro }}/setup.bash
python -m pip install pre-commit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will probably get problematic on 24.04, since python 3.11 doesn't allow to install pip packages without a venv by default (see e.g. https://stackoverflow.com/questions/75602063/pip-install-r-requirements-txt-is-failing-this-environment-is-externally-mana). We can keep that in mind for later, though...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it seemed that every step has its own vritual environment? this is why https://github.com/pre-commit/action/blob/main/action.yml#L13 didn't show any of the ROS packages before?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the hooks use their own venv (if they are python hooks). However, pre-commit itself is installed without a venv here. I am simply referring to the python -m pip install pre-commit command. This will not work on 24.04 (without any modifications to the systemat least). One way to resolve this (Doing this on my 23.10 machine for instance) would be to install pipx (available in apt) and then do pipx install pre-commit && pipx ensurepath

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then we will have more problems, for example also with the sphinx build I guess
https://github.com/ros-controls/control.ros.org/blob/51ff1372c4014845b4b3b7f9fd86644d05fa1db3/.github/workflows/sphinx-make-page.yml#L22-L32

let's keep it like this for now and remember it until rolling is targeting on 24.04 (which might be together with Jazzy release?)

pre-commit run --show-diff-on-failure --color=always --all-files --hook-stage manual
2 changes: 0 additions & 2 deletions .github/workflows/reusable-ros-tooling-source-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ jobs:
env:
# this will be src/{repo-owner}/{repo-name}
path: src/${{ github.repository }}
strategy:
fail-fast: false
steps:
- uses: ros-tooling/setup-ros@0.7.1
with:
Expand Down
49 changes: 49 additions & 0 deletions .github/workflows/reusable-update-pre-commit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: Update pre-commit
# Update pre-commit config and create PR if changes are detected
# author: Christoph Fröhlich <christoph.froehlich@ait.ac.at>

on:
workflow_call:

jobs:
auto_update_and_create_pr:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Install pre-commit
run: |
pip install pre-commit

- name: Auto-update with pre-commit
run: |
pre-commit autoupdate || true # Ignoring errors

- name: Check for changes
id: git_status
run: |
git diff --quiet && echo "::set-output name=changed::false" || echo "::set-output name=changed::true"

- name: There are changes
if: steps.git_status.outputs.changed == 'true'
run: |
echo "Files have changed"
git diff --exit-code || true

- name: No changes!
if: steps.git_status.outputs.changed == 'false'
run: |
echo "No changes detected"

- name: Create Pull Request
if: steps.git_status.outputs.changed == 'true'
uses: peter-evans/create-pull-request@v6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want to specify the author here, as well? Otherwise, the default author Defaults to the user who triggered the workflow run which might be misleading in some situations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd test this first without specifying it, because in the demos of the action the PR is also from the github action bot without having specified it.. let's see ;)

with:
token: ${{ secrets.GITHUB_TOKEN }}
branch: auto-update
commit-message: Auto-update with pre-commit
title: Bump version of pre-commit hooks
body: This pull request contains auto-updated files of pre-commit config.
delete-branch: true
12 changes: 12 additions & 0 deletions .github/workflows/update-pre-commit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: Auto Update pre-commit
# Update pre-commit config and create PR if changes are detected
# author: Christoph Fröhlich <christoph.froehlich@ait.ac.at>

on:
workflow_dispatch:
schedule:
- cron: '0 0 * * 0' # Run every Sunday at midnight

jobs:
auto_update_and_create_pr:
uses: ./.github/workflows/reusable-update-pre-commit.yml
30 changes: 18 additions & 12 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
repos:
# Standard hooks
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
rev: v4.5.0
hooks:
- id: check-added-large-files
- id: check-ast
Expand All @@ -36,7 +36,7 @@ repos:

# Python hooks
- repo: https://github.com/asottile/pyupgrade
rev: v3.4.0
rev: v3.15.1
hooks:
- id: pyupgrade
args: [--py36-plus]
Expand All @@ -49,40 +49,38 @@ repos:
args: ["--ignore=D100,D101,D102,D103,D104,D105,D106,D107,D203,D212,D404"]

- repo: https://github.com/psf/black
rev: 23.3.0
rev: 24.2.0
hooks:
- id: black
args: ["--line-length=99"]

- repo: https://github.com/pycqa/flake8
rev: 6.0.0
rev: 7.0.0
hooks:
- id: flake8
args: ["--extend-ignore=E501"]

# CPP hooks
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v15.0.6
rev: v17.0.6
hooks:
- id: clang-format
args: ['-fallback-style=none', '-i']

- repo: local
hooks:
- id: ament_cppcheck
name: ament_cppcheck
description: Static code analysis of C/C++ files.
stages: [commit]
entry: ament_cppcheck
entry: env AMENT_CPPCHECK_ALLOW_SLOW_VERSIONS=1 ament_cppcheck
language: system
files: \.(h\+\+|h|hh|hxx|hpp|cuh|c|cc|cpp|cu|c\+\+|cxx|tpp|txx)$

# Maybe use https://github.com/cpplint/cpplint instead
- repo: local
hooks:
- id: ament_cpplint
name: ament_cpplint
description: Static code analysis of C/C++ files.
stages: [commit]
entry: ament_cpplint
language: system
files: \.(h\+\+|h|hh|hxx|hpp|cuh|c|cc|cpp|cu|c\+\+|cxx|tpp|txx)$
Expand All @@ -94,7 +92,6 @@ repos:
- id: ament_lint_cmake
name: ament_lint_cmake
description: Check format of CMakeLists.txt files.
stages: [commit]
entry: ament_lint_cmake
language: system
files: CMakeLists\.txt$
Expand All @@ -105,7 +102,6 @@ repos:
- id: ament_copyright
name: ament_copyright
description: Check if copyright notice is available in all files.
stages: [commit]
entry: ament_copyright
language: system

Expand All @@ -128,8 +124,18 @@ repos:
# Spellcheck in comments and docs
# skipping of *.svg files is not working...
- repo: https://github.com/codespell-project/codespell
rev: v2.2.4
rev: v2.2.6
hooks:
- id: codespell
args: ['--write-changes']
exclude: CHANGELOG\.rst|\.(svg|pyc)$

- repo: https://github.com/python-jsonschema/check-jsonschema
rev: 0.28.0
hooks:
- id: check-github-workflows
args: ["--verbose"]
- id: check-github-actions
args: ["--verbose"]
- id: check-dependabot
args: ["--verbose"]
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

This repository holds reusable workflows for CI of the ros2_control framework.

It also build the full ros2_control stack once per day.
It also builds the full ros2_control stack once per day.

[![Rolling Stack Build](https://github.com/ros-controls/ros2_control_ci/actions/workflows/rolling-binary-build.yml/badge.svg)](https://github.com/ros-controls/ros2_control_ci/actions/workflows/rolling-binary-build.yml)
[![Iron Stack Build](https://github.com/ros-controls/ros2_control_ci/actions/workflows/iron-binary-build.yml/badge.svg)](https://github.com/ros-controls/ros2_control_ci/actions/workflows/iron-binary-build.yml)
Expand Down
Loading