-
-
Notifications
You must be signed in to change notification settings - Fork 185
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: Correctly record missing stages in index for merge conflicts #1907
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1907 +/- ##
==========================================
+ Coverage 97.77% 97.95% +0.17%
==========================================
Files 49 49
Lines 5261 5269 +8
==========================================
+ Hits 5144 5161 +17
+ Misses 117 108 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
One minor suggestion. Regarding the tests, indeed it's tricky to test... I trust you 😄
Previous tests already assert that the file is at least marked as unmerged, meaning the |
Fixes #1833.
I didn't add a test, because it would just be asserting that the Git commands do what they are supposed to, but feel free to ask for a test anyway.
I'm also not sure about the logic when a file did not exist in the previous template version, or was deleted in the latest template version: here I just skip the corresponding stage (2 when created, 3 when deleted) in the index, but maybe the whole index update (all 3 stages) could be skipped for such cases, not sure.
I rewrote the explanation comment above, reusing the wording in the SO answer, as I think it's much more readable than the previous comment.