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

Allow specifying a set of files/directories to lint #512

Closed
bcbernardo opened this issue May 2, 2022 · 7 comments
Closed

Allow specifying a set of files/directories to lint #512

bcbernardo opened this issue May 2, 2022 · 7 comments
Labels
enhancement New feature or request
Milestone

Comments

@bcbernardo
Copy link

Hi! Thanks for the great tool!

I have a feature request that would greatly improve my current workflow with reuse-tool, that includes a step in my pre-commit hooks that calls reuse lint against the project.

I think it would be more adequate for this use case if only staged files were checked. However, reuse lint currently has no way of restricting its checks to a specific set of files.

I understand reuse tool currently focus on compliance with reuse spec at the project level, but I think it would be nice it took a number of arguments to its command line, indicating which files to lint:

# lint only the given files
$ reuse lint foo.md bar.py

# running without arguments would fallback to the default behavior of checking all files in the current directory
$ reuse lint

When paired with a pre-commit hook, this API would not force the current contributor to make a big commit solving all licensing problems in the project, but would instead make sure that her contributions (only) have proper license and copyright information.

@carmenbianca
Copy link
Member

This functionality used to exist, but was removed during a refactoring, I believe.

Reintroducing this is a little bit tricky, because certain tests would have to be disabled when running the linter only against a few files. The code is already a bit of a spaghetti that is held together by unit and integration tests. Adding this as more spaghetti would be slightly tricky, but not impossible, but I'm starting to think that maybe some untangling is in order.

Thanks for the suggestion, though, @bcbernardo ! Adding this to TODO.

@carmenbianca carmenbianca added the enhancement New feature or request label May 3, 2022
@mxmehl mxmehl changed the title Allow specifying a set of files to lint Allow specifying a set of files/directories to lint Jun 2, 2022
@mxmehl mxmehl added this to the v1.1.0 milestone Jun 2, 2022
@mxmehl
Copy link
Member

mxmehl commented Jun 2, 2022

In today's maintainers call, we picked this as a feature for the next release.

Some questions are up for discussion:

  • Do want to integrate this in lint or make this a new subcommand like check or check-file?
  • Shall this then also display the found copyright/license for the given subset, or just work via an exit code?
  • Shall the output also contain a summary like the usual reuse lint, just limited to what's found in the subset? (Note: The last line should then definitely not be displayed)

What's certain is that the whole project scope must considered, so that the given file/directory is also recognised as compliant if it's covered in the dep5 file, with a separate .license file, or simply ignored in the VCS.

@bcbernardo
Copy link
Author

For all these open questions, my take is to try to get the closest experience possible to other widely used formatters & linters, such as black, isort and flake8.

Taking the mentioned tools as examples, I believe all of them:

  • Use the same subcommand for processing both the whole project (when no argument is given) or specific files or directories (when one or more paths are given as arguments).
  • Print a friendly and succinct message to stdout when no error is found; or a more detailed report about the inconsistencies found (or files changed) if that is the case.
  • Limit their reports to the information regarding only the given paths (if any).

Personally I find these choices quite sensible.

@koen-vg
Copy link

koen-vg commented Jun 8, 2022

I will also add that having the option to run reuse lint only against staged files seems to make a lot of sense for the pre-commit hook. I just ran into the issue that I wanted to make a commit but was blocked by the reuse tool due to some other unrelated and unstaged files which didn't have a license yet. This seems like an unnecessary annoyance.

Other than that, thanks for the great tool!

@nicorikken
Copy link
Member

nicorikken commented Jul 6, 2022

@mxmehl
Copy link
Member

mxmehl commented Aug 31, 2022

In today's call we clarified some more questions regarding this feature and its relation with other features we're working on:

  • It shall be incorporated in the lint subcommand, not a separate one
  • The output shall be similar to both the plain, human-readable reuse lint as well as reuse lint --json. However, we may need to strip some fields such as whether the project is compliant (as this requires the whole project scope) as well as the Unused Licenses or so. Instead, we may also add a comment as values of these fields indicating that these do not make sense when considering just a subset of the project.
  • However, it must consider the whole project scope in order to also respect a file in the subset being part of the DEP5 file.
  • The subset of files/directories shall be given by argument, so e.g. reuse lint src/*.py or reuse lint --json a.txt b.json, not with a flag such as --path.
  • We start working on this feature after Add --json flag to lint #183 and Add --verbose flag to lint, showing copyright/license info sources #256 got finished.

@carmenbianca
Copy link
Member

Implemented in #1055. It doesn't recursively do directories, so you'd need to use your shell's glob expansion for that.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants