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: allow multiple directories #29

Closed
wants to merge 1 commit into from

Conversation

SethFalco
Copy link
Contributor

@SethFalco SethFalco commented Sep 27, 2021

Currently, we're running the fpb-lint command 4 times to test each directory.

This change makes the command take a list of directories instead of just 1.
After this, we can update free-programming-books from having 4 separate run steps to just:

- - run: fpb-lint ./books/
- - run: fpb-lint ./casts/
- - run: fpb-lint ./courses/
- - run: fpb-lint ./more/
+ - run: fpb-lint ./books/ ./casts/ ./courses/ ./more/

The motivation is to collate the linting to a single process, output, and exit code.
This will make it simpler later when/if we make more use of the output, like suggesting fixes and automatic suggestions on GitHub.

By merging the 4 runs into 1, this also incidentally addresses an issue we have with GitHub Actions where the build fails early. If warnings were displayed in ./books/, it would fail and stop linting further files. This means there could be more warnings in ./casts/ but the contributor wouldn't know until after fixing the warnings from ./books/.


I added Commander as a dependency for 2 reasons:

  • CLI parsing is a pain, this is simpler than accounting for spaces, quotes, escapes (\), etc.
  • I'm hoping we can add more commands or arguments later. Namely, an argument to automatically apply fixes which can be used to automate suggestions in reviews.

@vhf
Copy link
Owner

vhf commented Oct 20, 2023

Merged and release 6f39fdb

@vhf vhf closed this Oct 20, 2023
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