Skip to content

Add dependabot, devcontainer and make cr-checker usable via pre-commit hooks#117

Merged
AlexanderLanin merged 15 commits into
eclipse-score:mainfrom
elektrobit-contrib:make-cr-checker-usable-via-pre-commit-hooks
Mar 6, 2026
Merged

Add dependabot, devcontainer and make cr-checker usable via pre-commit hooks#117
AlexanderLanin merged 15 commits into
eclipse-score:mainfrom
elektrobit-contrib:make-cr-checker-usable-via-pre-commit-hooks

Conversation

@lurtz
Copy link
Copy Markdown
Contributor

@lurtz lurtz commented Mar 5, 2026

This is a collection of improvements among these the pre-commit-hook.yaml is the one I care most. It allows to check copyright headers without bazel. The solution is not yet perfect, but works good enough for a simple python script.

The other stuff (dependabot, devcontainer) is convenience and automation.

@lurtz lurtz marked this pull request as ready for review March 6, 2026 10:04
@lurtz lurtz changed the title Add dependabot, devcontainer and make cr checker usable via pre commit hooks Add dependabot, devcontainer and make cr-checker usable via pre-commit hooks Mar 6, 2026
Comment thread cr_checker/tool/pre-commit_wrapper Outdated
@lurtz lurtz force-pushed the make-cr-checker-usable-via-pre-commit-hooks branch 2 times, most recently from b969a24 to 40657f5 Compare March 6, 2026 10:35
@lurtz lurtz force-pushed the make-cr-checker-usable-via-pre-commit-hooks branch from 40657f5 to f84bd2e Compare March 6, 2026 11:19
@AlexanderLanin AlexanderLanin requested a review from Copilot March 6, 2026 11:20

This comment was marked as outdated.

Comment thread .devcontainer/Dockerfile Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rs]
// *******************************************************************************
// Copyright (c) {year} Contributors to the Eclipse Foundation
// Copyright (c) {year} {author}
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

Changing the Rust header template to use {author} makes has_copyright() treat the author portion as a wildcard (it formats {author} as .* when building the regex), so Rust files with any author string will now be considered compliant. If the intent is to enforce a specific author (e.g., from config.json), keep the literal author in the template or update has_copyright() to substitute the configured author instead of .*.

Suggested change
// Copyright (c) {year} {author}
// Copyright (c) {year} Contributors to the Eclipse Foundation

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the other templates are done the same way

Comment on lines 508 to 512
if offset > 0 and offset != byte_array:
LOGGER.error("Invalid offset value: %d, expected: %d", offset, byte_array)
LOGGER.error(
"%s: Invalid offset value: %d, expected: %d", path, offset, byte_array
)
return False
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

fix_copyright rejects offsets that include trailing newline(s) after a shebang because it requires offset == len(first_line) (byte length of only the first line). But detect_shebang_offset() explicitly counts trailing \n/\r, so process_files(..., --fix) will fail to fix files whose shebang line is followed by blank line(s). Consider preserving exactly offset bytes (read/write that prefix) or relaxing the validation to accept offsets greater than the first-line byte length and preserve those extra newline bytes as well.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

might pick this up in future work. At the moment the logging shall help in finding files, which make it fail for manual fixes.

Comment thread .pre-commit-hooks.yaml
entry: cr_checker/tool/pre-commit_wrapper --extensions h hpp c cpp rs rst py sh bzl ini yml yaml BUILD bazel --fix
# this would be the better language implementation, but struggle with python tooling
# language: python
language: unsupported_script
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

language: unsupported_script is not a valid pre-commit hook language (pre-commit supports e.g. script, system, python, etc.). This will cause pre-commit to fail loading the hook manifest/config. Use language: script (if pre-commit_wrapper is executable) or language: system with an explicit python3 ... entry.

Suggested change
language: unsupported_script
language: script

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

script is the same, but obsolete language: https://pre-commit.com/#unsupported_script

Comment thread .pre-commit-config.yaml
entry: cr_checker/tool/pre-commit_wrapper --extensions h hpp c cpp rs rst py sh bzl ini yml yaml BUILD bazel --fix
# this would be the better language implementation, but struggle with python tooling
# language: python
language: unsupported_script
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

language: unsupported_script is not a valid pre-commit hook language, so the local hook configuration will error when running pre-commit install / pre-commit run. Switch to language: script (if the wrapper is executable) or language: system and invoke python3 explicitly in entry.

Suggested change
language: unsupported_script
language: script

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

script is the same, but obsolete language: https://pre-commit.com/#unsupported_script

@AlexanderLanin AlexanderLanin merged commit 31ff8ee into eclipse-score:main Mar 6, 2026
8 checks passed
@lurtz lurtz deleted the make-cr-checker-usable-via-pre-commit-hooks branch March 6, 2026 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

4 participants