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

Script documentation: run addheader on new files via pre-commit #534

Open
mxmehl opened this issue Jun 2, 2022 · 5 comments · May be fixed by #761
Open

Script documentation: run addheader on new files via pre-commit #534

mxmehl opened this issue Jun 2, 2022 · 5 comments · May be fixed by #761
Assignees
Labels
blocked This issue/pull request is blocked by anoter documentation Missing or wrong documentation

Comments

@mxmehl
Copy link
Member

mxmehl commented Jun 2, 2022

@carmenbianca desires a feature that somehow automatically takes care of adding REUSE headers to new files, ideally via a pre-commit hook. Ideally, this would also be configurable per repo, as mentioned in #68.

The tool maintainers agreed that this should not be included in the tool itself, because there are a number of difficulties:

  • Users of this feature may overwrite existing headers (see the discussion in reuse addheader destroys content #338)
  • They may also introduce false information that does not match the actual copyright/licensing of the project or file that's added. Example: a users pastes a file from a different project under different copyright/license that does not carry REUSE information yet. Such an automatic hook would out false info on this file!

Instead, we should document how developers could do this in the scripts documentation. while making the aware of the potential issues.

@kbroch-rivosinc
Copy link
Contributor

I am wondering if there is a possibility to reconsider this:

The tool maintainers agreed that this should not be included in the tool itself, because there are a number of difficulties:

I'm finding this rivosinc#1 very useful on projects I convert to reuse compliance. After I go through the manual process of annotating the headers, I know the rule(s) of the project and I want to apply those in the future by setting them up in the .pre-commit-config.yaml file. IMO this automation is less error prone than relying on humans to apply reuse annotate manually based of some documentation. (and any script I create would simple apply the same rules as the pre-commit hook but add more code and complexity to my projects).

@nicorikken
Copy link
Member

@kbroch-rivosinc thanks for commenting and writing that pre-commit hook! The main concern with automated annotations is the risk of it being incorrect and eventually being published as the truth.

Looking at the PR you linked, it seems that the user still needs to configure the pre-commit hook to add copyright and license properties. One could argue that this is similar to some wildcard matching based on a .reuse/dep5 file where the annotation is also automatic. Actually using the pre-commit hook would make it even more explicit because it would be pre-commit (!) so there is a manual review before committing. This is less explicit when committing files in certain directories.

Still I think we should add plenty of warnings to communicate that there is no guarantee for correctness if applied automatically.

Interested to hear what others think.

@nicorikken
Copy link
Member

Regarding the pre-commit hook, having a mechanism for defaults would be necessary, as discussed in #68 (comment) because that would allow users to set their own default annotations. This is a better practice than setting project-specific default annotations.

So we consider annotation defaults as discussed in #68 a dependency for moving this forward. But we think then having a pre-commit hook with annotations can make sense. We agree that having this mechanism in place would be a great usability improvement.

@mxmehl mxmehl added the blocked This issue/pull request is blocked by anoter label Mar 9, 2023
@MarcelKoch
Copy link

Hi, I just wanted to add that we would also like to use this kind of automation (ideally the pre-commit hook) in our ginkgo project (ginkgo-project/ginkgo#1460). Currently, we add a local hook for that.

@carmenbianca
Copy link
Member

This is still on the todo list. With a little luck it will be worked on in the coming months.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue/pull request is blocked by anoter documentation Missing or wrong documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants