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

feat: lint-file subcommand #1055

Merged
merged 18 commits into from
Sep 10, 2024
Merged

feat: lint-file subcommand #1055

merged 18 commits into from
Sep 10, 2024

Conversation

klmcadams
Copy link
Contributor

@klmcadams klmcadams commented Aug 1, 2024

Added lint-file subcommand to allow people to lint specified file(s). In the command line, you can run reuse lint-file <files>. For example, reuse lint-file src/reuse/download.py src/reuse/_annotate.py. If you only run reuse lint-file with no files listed, then it will lint all files (as if you were just running reuse lint).

Let @SMoraisAnsys or I know if you have any questions or concerns! This PR addresses the suggestions in #903.

Here is a picture of running reuse lint-file in the command line with a file that doesn't exist (dne_file.py), two files that exist and are compliant, and one file that exists but is not compliant (empty_file.py):

lint_files_example

  • Added a change log entry in changelog.d/<directory>/.
  • Added self to copyright blurb of touched files.
  • Added self to AUTHORS.rst.
  • Wrote tests.
  • Documented my changes in docs/man/ or elsewhere.
  • My changes do not contradict
    the current specification.
  • I agree to license my contribution under the licenses indicated in the
    changed files.

@carmenbianca
Copy link
Member

Hi @klmcadams ! Thank you so much for this PR. Unfortunately I do not have time to review it this or next week.

@klmcadams
Copy link
Contributor Author

@carmenbianca that's okay! Thanks for letting me know

Copy link
Member

@carmenbianca carmenbianca left a comment

Choose a reason for hiding this comment

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

I did a cursory review. There's a number of things that need to change. Because I have time and this feature has some priority for me, I will work on patching this PR up.

Thank you @klmcadams !

.pre-commit-hooks.yaml Outdated Show resolved Hide resolved
src/reuse/project.py Outdated Show resolved Hide resolved
src/reuse/report.py Outdated Show resolved Hide resolved
tests/test_lint.py Outdated Show resolved Hide resolved
@klmcadams
Copy link
Contributor Author

@carmenbianca Thanks for the review! Let me know if I can help in any way

@carmenbianca
Copy link
Member

carmenbianca commented Sep 6, 2024

Rebased on top of main to fix a problem I had (#1067) while testing, and patched this up in one rather big commit.

Because the behaviour of lint-file is very slightly esoteric (as are so many things in reuse…), I will need to set aside some additional time to improve the man page.

Thanks a lot for the template to work on top of @klmcadams ! You had a lot of the right ideas.

If you have time, I would love a code review on this. If not, that is very understandable :) It's taken me a month to respond to you, after all.

edit: Of course the tests fail on CI and not on my computer! I'll need to fix that, too.

Signed-off-by: Carmen Bianca BAKKER <[email protected]>
This worked on my machine, but not on the CI. I'm convinced that my
filesystem (btrfs) iterates over files very differently compared to
ext4.

Signed-off-by: Carmen Bianca BAKKER <[email protected]>
Signed-off-by: Carmen Bianca BAKKER <[email protected]>
Copy link
Contributor

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

LGTM. You just need to remove the "Unused licenses" section from reuse-lint-file.rst.

docs/man/reuse-lint-file.rst Outdated Show resolved Hide resolved
Signed-off-by: Carmen Bianca BAKKER <[email protected]>
Signed-off-by: Carmen Bianca BAKKER <[email protected]>
Signed-off-by: Carmen Bianca BAKKER <[email protected]>
@carmenbianca
Copy link
Member

Perfect, merging! I'll get a release of this out the door some time soon. Thank you @klmcadams and @SMoraisAnsys !

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.

3 participants