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.

Sign up for GitHub

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
+124 −15
Merged

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

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
7508264
Create ci-format.yml
christophfroehlich Feb 18, 2024
54a3d05
Create a reusable workflow
christophfroehlich Feb 18, 2024
df325c3
Fix typo
christophfroehlich Feb 18, 2024
d069b1d
Rename workflows and remove system hooks
christophfroehlich Feb 18, 2024
4669354
Remove commit stage from ros linters
christophfroehlich Feb 18, 2024
913e0e2
Install proper system hooks for ament_linters
christophfroehlich Feb 18, 2024
b2d414b
Fix typo
christophfroehlich Feb 18, 2024
bcb7a05
Try not to give explicit python version
christophfroehlich Feb 18, 2024
b691547
Update pre-commit hook versions
christophfroehlich Feb 18, 2024
259c54d
Remove redundant job name
christophfroehlich Feb 18, 2024
d09ae58
Set AMENT_CPPCHECK_ALLOW_SLOW_VERSIONS directly with pre-commit config
christophfroehlich Feb 18, 2024
17f3c52
Source ROS workspace
christophfroehlich Feb 18, 2024
b49f7ee
Run pre-commit manually instead of composite action
christophfroehlich Feb 19, 2024
1e8f742
Update comments
christophfroehlich Feb 19, 2024
56a8303
Add linters for github actions and workflows
christophfroehlich Feb 19, 2024
5b0c6fd
Use fallback-style to clang-format
christophfroehlich Feb 19, 2024
e70f769
Add pre-commit auto-update workflows
christophfroehlich Feb 19, 2024
adcac78
Add os_name input
christophfroehlich Feb 20, 2024
92072da
Fix os_name
christophfroehlich Feb 21, 2024
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
View file
Open in desktop
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
28 changes: 28 additions & 0 deletions .github/workflows/reusable-pre-commit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
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

jobs:
pre-commit:
runs-on: ubuntu-latest
christophfroehlich marked this conversation as resolved.
Show resolved Hide resolved
steps:
- uses: actions/checkout@v4
- uses: ros-tooling/[email protected]
- 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
View file
Open in desktop
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/[email protected]
with:
Expand Down
49 changes: 49 additions & 0 deletions .github/workflows/reusable-update-pre-commit.yml
View file
Open in desktop
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 <[email protected]>

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.

ah, you are right: https://github.com/orgs/community/discussions/47855#discussioncomment-7061183

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
View file
Open in desktop
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 <[email protected]>

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
View file
Open in desktop
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
View file
Open in desktop
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