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.

Sign up for GitHub

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

Jump to bottom

Refactors credentials callback on remote to make pushes authenticate #438

Closed
wants to merge 1 commit into from
Closed

Refactors credentials callback on remote to make pushes authenticate #438

wants to merge 1 commit into from

Conversation

eldavido
Copy link

#437

There's a single test break in this:

======================================================================
FAIL: test_remote_refcount (test.test_remote.RepositoryTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dalbrecht/pygit2/test/test_remote.py", line 189, in test_remote_refcount
    self.assertEqual(start, end)
AssertionError: 2 != 3

----------------------------------------------------------------------
Ran 228 tests in 9.560s

FAILED (failures=1)

It looks like the patch is causing the refcount to get thrown. If someone could take a look at why this is happening, we can get this merged and pushes should work with credentials.

@carlosmn
Copy link
Member

The reason is that you removed a self._self_handle = None assignment, which is necessary to break the cyclic reference which python cannot break itself. That is what's keeping #431 from getting merged, which attempts to solve this problem as well.

@jdavid
Copy link
Member

jdavid commented Nov 3, 2014

So issue #437 and PRs #431 and #438 are about the same problem? Would be nice to get this fixed for the upcoming release.

@carlosmn
Copy link
Member

carlosmn commented Nov 3, 2014

Yeah, I was hoping @DrWong would get back to us on the handle issue, but I can squash the fix in and push that to master if you'd like. We can still have a refactoring after the pushing bit is fixed.

@jdavid
Copy link
Member

jdavid commented Nov 3, 2014

@carlosmn that would be cool

@jdavid
Copy link
Member

jdavid commented Nov 3, 2014

So I understand this is fixed.. closing

@jdavid jdavid closed this Nov 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eldavido @carlosmn @jdavid