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

add a trailing whitespace lint #1174

Merged
7 commits merged into from
Feb 24, 2021
Merged

add a trailing whitespace lint #1174

7 commits merged into from
Feb 24, 2021

Conversation

ghost
Copy link

@ghost ghost commented Feb 24, 2021

I was curious about what util/exercise and noticed the Cargo.toml has trailing whitespace.
So I took the opportunity to add a lint in CI too.
This is an example failure.

efx added 3 commits February 24, 2021 15:09
I could not figure out how to use `-or` with `find` and `-exec` so am
only doing this for `.toml` files.
It would be nice to do so for `.sh` files.
@@ -74,7 +77,7 @@ jobs:

# stolen from https://raw.githubusercontent.com/exercism/github-actions/main/.github/workflows/shellcheck.yml
shellcheck:
name: shellcheck internal tooling
name: shellcheck internal tooling lint
Copy link
Author

Choose a reason for hiding this comment

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

Renamed this for consistency. I should also rename our markdown_lint.sh to lint_markdown.sh. It seems most of our internal tooling follows a category-subcategory naming scheme.

Copy link
Member

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

Good idea, makes sense.

bin/lint_trailing_spaces.sh Outdated Show resolved Hide resolved
@@ -102,7 +102,7 @@ fn process_matches(matches: &ArgMatches<'_>) -> Result<()> {
}

("", None) => {
println!("No subcommand was used.\nUse init_exercise --help to learn about the possible subcommands.");
println!("No subcommand was used.\nUse exercise --help to learn about the possible subcommands.");
Copy link
Member

Choose a reason for hiding this comment

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

This change makes sense, but seems pretty unrelated to the rest of this PR.

Copy link
Author

Choose a reason for hiding this comment

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

I agree. How do you feel about documenting something like a Unix philosophy for our pull requests?

Keep it small.
One logical change or something.
I can take a stab for #1111.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, "one logical change" is the best expression of what I want to see in a PR. It's not necessarily small--there have been good, useful PRs which added hundreds of lines of code and took months to resolve (looking at you, #653)--but life is easier when PRs are focused.

@ghost ghost changed the title add a trailing whitespace lint, and util crate typo fix add a trailing whitespace lint Feb 24, 2021
@ghost ghost merged commit 188d7fe into main Feb 24, 2021
@ghost ghost deleted the util-exercise-docs-fix branch February 24, 2021 21:44
This pull request was closed.
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