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

Fix: Pre-commit hook includes untracked files in commits #1041

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

christolis
Copy link
Member

Fixes #1039.

With this pull request, untracked files do not get included in commits handled by the pre-commit hook.

Screenshots
image
image
image

@christolis christolis requested review from a team as code owners March 2, 2024 10:39
@christolis christolis self-assigned this Mar 2, 2024
@christolis christolis added bug Something isn't working priority: critical labels Mar 2, 2024
@Zabuzard Zabuzard self-requested a review March 7, 2024 07:46
@Zabuzard
Copy link
Member

Zabuzard commented Mar 7, 2024

I dont quite get the point of this. Untracked files should NOT be automatically included in commits. Otherwise you can not keep files untracked, which can be on purpose (I do this quite often).
@marko-radosavljevic for review please

@Zabuzard Zabuzard requested review from marko-radosavljevic and removed request for Zabuzard and a team March 7, 2024 07:47
Copy link
Contributor

@marko-radosavljevic marko-radosavljevic left a comment

Choose a reason for hiding this comment

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

Blocked until I review and test it, per Daniel's request.

@christolis
Copy link
Member Author

For reference the -A option, found in the original code, works like this (source, emphasis mine):

Update the index not only where the working tree has a file matching but also where the index already has an entry. This adds, modifies, and removes index entries to match the working tree.

If no is given when -A option is used, all files in the entire working tree are updated (old versions of Git used to limit the update to the current directory and its subdirectories).

The -u option in the PR works like this (source, emphasis mine):

Update the index just where it already has an entry matching . This removes as well as modifies index entries to match the working tree, but adds no new files.

If no is given when -u option is used, all tracked files in the entire working tree are updated (old versions of Git used to limit the update to the current directory and its subdirectories).

Copy link

@tj-wazei tj-wazei left a comment

Choose a reason for hiding this comment

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

I have tested and reproduced the issue. It is indeed a bug.

In my test, the following output was provided when running git status:

On branch bug-test
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   application/src/main/java/org/togetherjava/tjbot/Application.java

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        application/src/main/java/org/togetherjava/tjbot/BugTest.java

After running git commit -m "test"

This is the result of git status:

On branch bug-test
nothing to commit, working tree clean

The output from the pre-commit hook shows:

****Done pre-commit hooks****
[bug-test 96a2cb1] test
 2 files changed, 6 insertions(+)
 create mode 100644 application/src/main/java/org/togetherjava/tjbot/BugTest.java

Clearly, we can see it's commiting untracked files.

@Zabuzard Zabuzard dismissed marko-radosavljevic’s stale review March 25, 2024 07:50

took too long to review

@Zabuzard Zabuzard merged commit a1ef33e into Together-Java:develop Mar 25, 2024
9 checks passed
@Zabuzard Zabuzard mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: critical
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pre-commit hook includes untracked files in commits
6 participants