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

Change how check-file-format.sh checks changed files #130

Merged
merged 7 commits into from
Sep 26, 2023

Conversation

regularfry
Copy link
Contributor

Description

Prior to this patch, the git diff command used to identify which files to check for editorconfig compliance caught too many files, and did the expensive part of the check inside the per-file loop.

By lucky chance, the editorconfig-checker container image has a git binary installed, so we can move the entire loop inside a single container invocation.

Fixes #129.

Context

It makes the editorconfig check, which is called in a pre-commit hook, fast.

Type of changes

  • Refactoring (non-breaking change)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would change existing functionality)
  • Bug fix (non-breaking change which fixes an issue)

Copy link
Contributor

@stefaniuk stefaniuk left a comment

Choose a reason for hiding this comment

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

Just a suggestion to do a bit of refactoring as we go.

scripts/githooks/check-file-format.sh Outdated Show resolved Hide resolved
scripts/githooks/check-file-format.sh Outdated Show resolved Hide resolved
scripts/githooks/check-file-format.sh Show resolved Hide resolved
@regularfry
Copy link
Contributor Author

Think that's done now. We've got the modes as described here, and I've added a dry_run option because otherwise seeing the list of files is a right pain.

regularfry and others added 4 commits September 23, 2023 08:35
Prior to this patch, the `git diff` command used to identify which files to check for editorconfig compliance caught too many files, and did the expensive part of the check inside the per-file loop.

By lucky chance, the editorconfig-checker container image has a `git` binary installed, so we can move the entire loop inside a single container invocation.

Fixes #129.
scripts/githooks/check-file-format.sh Show resolved Hide resolved
scripts/config/pre-commit.yaml Show resolved Hide resolved
scripts/githooks/check-file-format.sh Outdated Show resolved Hide resolved
scripts/githooks/check-file-format.sh Outdated Show resolved Hide resolved
scripts/githooks/check-file-format.sh Outdated Show resolved Hide resolved
@regularfry
Copy link
Contributor Author

Do we know how the 18af9e0 force-push happened?

@stefaniuk
Copy link
Contributor

Do we know how the 18af9e0 force-push happened?

Yes, I brought this branch in sync with main as it was outdated. Sorry.

Copy link
Contributor

@stefaniuk stefaniuk left a comment

Choose a reason for hiding this comment

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

LGTM. thanks! merging...

@stefaniuk stefaniuk added this pull request to the merge queue Sep 26, 2023
Merged via the queue into main with commit 3928625 Sep 26, 2023
22 checks passed
@stefaniuk stefaniuk deleted the editorconfig-loop branch September 26, 2023 13:31
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.

File selection for check-file-format.sh seems over-broad and potentially misconfigured
2 participants