kblincoe / VisualGit_SE701_2019_1

1 stars 0 forks source link

Fixes #55 - Pushing changes before committing crashes application #228

Closed SheepySean closed 5 years ago

SheepySean commented 5 years ago

Related Issue/Keyword:

Fixes #55

Description:

Prior to this commit. It was possible to push to the remote without having any file changes comitted. This could cause crashing to the application by needlessly calling the node-git API and always was pretty bad UX. This PR introduces functionality to check if there are any changes that need to be pushed to the repo before pushing them. If there are 0 then the push is ignored as it is redundant. It's important to note here, this functionality could be leveraged on the ui side of things to inform the user how many commits are not pushed.

BIG NOTE: This PR does not unfortunately fix all pushing errors as the node-git API can actually cause segmentation faults with its threading policy when pushing so that's really fun. I've spent many hours researching this and it appears others having the problem, and its not quite fixable. Good news, if node-git fixes their stuff then this would also be fixed. This PR predominately helps to guard the pushing functionality to when its only needed. https://github.com/nodegit/nodegit/issues/1591

Testing:

Steps for manual testing:

  1. Open a repo that has a remote registered with it
  2. Immediately click push and note the dialog saying there are no changes to push.
  3. Make a change, click push again and note the dialog saying there are no changes to push.
  4. Commit the change and click push. The push will continue as per normal.

Checklist:

twchen97 commented 5 years ago

Doing the manual tests, the dialog showing no changes to push is looking good and I can do steps 1, 2, and 3 successfully. For some reason my modified files aren't showing up on VisualGit anymore, and it's not letting me commit so I can't test the last step for manual testing.

It could be just my machine, but are you getting this problem as well? The issue is probably out of your scope for this PR but I don't think I can finish the manual testing without that being resolved first.

SheepySean commented 5 years ago

Doing the manual tests, the dialog showing no changes to push is looking good and I can do steps 1, 2, and 3 successfully. For some reason my modified files aren't showing up on VisualGit anymore, and it's not letting me commit so I can't test the last step for manual testing.

It could be just my machine, but are you getting this problem as well? The issue is probably out of your scope for this PR but I don't think I can finish the manual testing without that being resolved first. @twchen97

I am also getting those issues, and also on master so its not this feature breaking it for whatever reason.

My suggestion for testing is using the command line to commit files. So testing wise this would be for step 4: