-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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 staged file renamed with changes showing no diff #17467
Conversation
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.
@ssigwart Thanks for this contribution. I apologize for the long delay in review (small team, pr's are as we can get to them). I understand if you do not want to pick this back up after so much time. But if you do, here is some feedback:
I believe it is fairly non-invasive approach to having better messaging and appears to work as expected. I have left some code-base stylistic comments. Also, I can tell that if the CI had triggered, the linter would have failed. Thus, to get this PR another review, we will need to bring it up to date with development, address the pr suggestions, and run yarn:lint:fix. At that point, I will do more testing.
As said above, totally understand if you do not want to pick this back up. If that is the case we will close it in a couple of weeks.
- Closes desktop#6014 - Fixes the case where a staged file is moved and modified. Previously, it would say it was moved with no changes.
83db3b6
to
bf8f6d1
Compare
Thanks, @tidy-dev. I rebased and applied the updates you suggested. The GitHub actions don't run without approval for me, but I ran |
@ssigwart We have some failing unit test since the status module has changed. Can you address those? To run unit tests, use |
@tidy-dev, I fixed them. Thanks for the pointers on how to run the tests that failed. |
@tidy-dev, I noticed that the unit tests ran today and there were some lint errors, so I ran |
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.
Works as expected. Thank you!
Thanks for reviewing and merging, @tidy-dev! |
Description
Fixes the case where a staged file is moved and modified. Previously, it would say it was moved with no changes.
Screenshots