-
-
Notifications
You must be signed in to change notification settings - Fork 237
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
Don't commit files when only LF/CRLF changes #265
Conversation
Thanks for the PR @ZeroRin. I will take a closer look over the next few days. Big thanks for also fixing the test case. |
Update test name to make it clear that the Action no longer fails to detect CRLF changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the name of the test and added some comment for future me.
@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. |
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:
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. |
Thanks @ZeroRin. Gonna merge this PR now and tag a new version.
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 |
Previously I tried to solve this by switching the order for
add
andstatus
, which results in some side effects.Here I tried to solve the issue by adding a
diff
check right beforecommit
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.