Skip to content
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

Correctly stash with submodules #27

Merged
merged 3 commits into from
Feb 11, 2016
Merged

Correctly stash with submodules #27

merged 3 commits into from
Feb 11, 2016

Conversation

Javex
Copy link

@Javex Javex commented Jun 5, 2015

I had a problem where a submodule has uncommitted changes. git up would choke on this, because is_dirty reports True but there isn't actually a change (the change count is 0, when correctly calculated, see below). Thus stashing would store an empty stash. Then on unstashing it fails because there is no stash. I provided gitpython-developers/GitPython#294 to GitPython to introduce a new argument that allows to ignore submodules when asking for is_dirty. This PR uses that change then.

Finally, I noticed the calculation of changes is off: If no change is made, the count would still be 1. Now it's correctly 0 (not something that would actually appear in reality, I guess).

Note that the PR on GitPython needs to be merged first. Beyond that, the version might need a bump in setup.py, but I guess you know that ;-)

Javex added 2 commits June 5, 2015 11:43
Submodule changes cannot be stashed and thus should not be considered when
stashing changes. Requires gitpython-developers/GitPython#294 to be applied
first.
The old version would consider an empty string as "1 change" where it should be
0. This change is able to determine a 0 count.
@msiemens
Copy link
Owner

msiemens commented Jun 5, 2015

Thanks for the PR! I'll merge it as soon as the change in GitPython gets approved.

@Javex
Copy link
Author

Javex commented Jun 10, 2015

In case you haven't seen, the PR has been merged, though I don't know whether you want to wait until it has also been released as well.

@msiemens
Copy link
Owner

Yeah, it's propably best to wait for a new release of GitPython. Unfortunately its mailing list doesn't seem very active so I'll try to monitor its GitHub project for the new release.

The pull request gitpython-developers/GitPython#294 has been merged but the API
was changed slighty. This commit reflects this change so that it works again.
@Javex
Copy link
Author

Javex commented Jun 11, 2015

I have updated the changes so that the changed API resulting from merging the PR are reflected here as well. Without this, this PR would break

@msiemens msiemens force-pushed the master branch 7 times, most recently from c6a99f0 to 4107474 Compare August 17, 2015 17:57
@msiemens msiemens merged commit 46b0bd3 into msiemens:master Feb 11, 2016
@msiemens
Copy link
Owner

It's been a while, but a new version of GitPython that includes your patch has finally been released. So your PR is now merged, I'll try to set up some test cases and then we can release an update of PyGitUp, too.

msiemens added a commit that referenced this pull request Feb 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants