search

stefanzweifel / git-auto-commit-action

Automatically commit and push changed files back to GitHub with this GitHub Action for the 80% use case.
MIT License
2.07k stars 231 forks source link

Don't commit files when only LF/CRLF changes #265

Closed ZeroRin closed 2 years ago

ZeroRin commented 2 years ago

Previously I tried to solve this by switching the order for add and status, which results in some side effects. Here I tried to solve the issue by adding a diff check right before commit so other behaviors are not affected.

Note that the testcase for crlf in the original repo does not change line ending properly, so I also updated the test. The original action fails on this test while the modified version passes all the tests.

Related to #241.

stefanzweifel commented 2 years ago

Thanks for the PR @ZeroRin. I will take a closer look over the next few days.

Big thanks for also fixing the test case.

stefanzweifel commented 2 years ago

@ZeroRin I have one last question, but I struggle to formulate it correctly:

The changes made in this PR … does this represented the behaviour one would expect?

If I understood everything correctly, the Action now no longer creates a commit, if CRLF changes/differs. Basically all I want to know is why we need to make this change.

ZeroRin commented 2 years ago

@ZeroRin I have one last question, but I struggle to formulate it correctly:

The changes made in this PR … does this represented the behaviour one would expect?

If I understood everything correctly, the Action now no longer creates a commit, if CRLF changes/differs. Basically all I want to know is why we need to make this change.

setting autocrlf means that one intentionally want all line endings in the repository to be LF, and ask git to manage it automatically. switching line ending style of a local file should not be considered as a change.

Actually I feel the current pipeline is a bit weird. We decide whether to do a commit at the very beginning, and hoping the following add stage does not break it. I believe that the dirty check should just be done right before the commit stage.

just come up with another example that would break the original action:

touch "temp"
git add .
rm "temp"

Here I created a repo with an unstaged change that reverts the staged change. The repo is considered dirty during the beginning check, but after git add it sudennly becomes clean, and would raise an error during commit stage. In contrast, I cannot think of a situation that we have to check dirtiness before add.

stefanzweifel commented 2 years ago

Thanks @ZeroRin. Gonna merge this PR now and tag a new version.

just come up with another example that would break the original action:

That example wouldn't work with this Action. There is no way for users to delete stuff between "stage files" and "create and push files". (Maybe through git-hooks, but that seems far fetched)

Anyway. :D Thanks again for you contribution!