Skip to content

[beman-tidy] Created file.copyright checks (dry-run & fix-inplace) | Added multiple_files_base_check#219

Draft
AndreiDurlea wants to merge 6 commits intobemanproject:mainfrom
AndreiDurlea:origin/andreidurlea-issue57
Draft

[beman-tidy] Created file.copyright checks (dry-run & fix-inplace) | Added multiple_files_base_check#219
AndreiDurlea wants to merge 6 commits intobemanproject:mainfrom
AndreiDurlea:origin/andreidurlea-issue57

Conversation

@AndreiDurlea
Copy link

Solved issue - #57
Introduced a new infrastructure for running FileBaseCheck's across multiple source files, while registering only one (main) check, that passes if all its checks pass, and fails otherwise.

Changes:

  • Added MultipleFilesBaseCheck as a new base class to support checks that need to operate on all source files in a repository, handling iteration through files, logging, and check&fix logic.
  • Implemented File.copyright

TODO:

  • Support block comments for File.copyright (should it?)
  • More tests (only 4 so far)

@AndreiDurlea
Copy link
Author

@neatudarius can you take a look at this?

@neatudarius neatudarius self-assigned this Jan 27, 2026
Copy link
Member

@neatudarius neatudarius left a comment

Choose a reason for hiding this comment

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

Thanks @AndreiDurlea for 1) interest in Beman Project and for 2) this big contribution!

Your PR actually delivers the functionality we need, but I have some suggestions related to the style of code, file organization, and new APIs added:

@@ -1,10 +1,10 @@
version = 1
revision = 2
revision = 3
Copy link
Member

Choose a reason for hiding this comment

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

Is this change needed?

Copy link
Author

Choose a reason for hiding this comment

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

No, it was accidental when syncing deps. Reverting it

return True


@register_beman_standard_check("file.copyright")
Copy link
Member

Choose a reason for hiding this comment

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

if "SPDX-License-Identifier:" in line:
spdx_index = i
break

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

it's validation of result from previous loop

return True

def fix(self):
try:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
try:

return True

spdx_index = -1

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

if spdx_index == -1:
return True

spdx_line = lines[spdx_index].strip()
Copy link
Member

Choose a reason for hiding this comment

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

duplicate code

from .base_check import BaseCheck


class MultipleFilesBaseCheck(BaseCheck):
Copy link
Member

Choose a reason for hiding this comment

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

Final step, let's rename this to BatchFileBaseCheck and move into base/file_base_check.py. We don't need a new file, it's actually connected to FileBaseCheck.

@neatudarius
Copy link
Member

neatudarius commented Jan 30, 2026

image Also check the failed tests on CI

AndreiDurlea and others added 3 commits February 1, 2026 20:47
Co-authored-by: Darius Neațu <neatudarius@gmail.com>
Co-authored-by: Darius Neațu <neatudarius@gmail.com>
"source" to cpp

Co-authored-by: Darius Neațu <neatudarius@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants