search

gollum / rugged_adapter

Adapter to make gollum use Rugged (libgit2) at the backend.
MIT License
36 stars 27 forks source link

Upgrade Rugged to 0.24.0 #17

Closed hiroponz closed 8 years ago

hiroponz commented 8 years ago

This is a beta version but the maintainers mentioned that it's solid and they use it at GitHub. And it needs the revert commit functionality from this version to resolve #15 .

bartkamphorst commented 8 years ago

Is there a timeline for the 0.24 non-beta release?

hiroponz commented 8 years ago

FYI. https://github.com/libgit2/libgit2/issues/3626

bartkamphorst commented 8 years ago

@hiroponz Version 0.24 was just released. Care to adapt the PR?

hiroponz commented 8 years ago

@bartkamphorst I have upgraded the gem dependency.

hiroponz commented 8 years ago

The build failed, but the last build have already failed.

hiroponz commented 8 years ago

@bartkamphorst The build passed, but it is temporary fix. Can you take a look?

bartkamphorst commented 8 years ago

The temporary spec fix seems fine to me. Have you tried running the gollum-lib specs using rugged v. 0.24?

hiroponz commented 8 years ago

1 error occurred at running the gollum-lib specs using rugged v.0.24.0, but also occurred on current rugged_adapter master(v.0.23.3). The error message is like the following.

Error: test_push_and_pull(Pushing_and_pulling): NoMethodError: undefined method `save' for #<Rugged::Remote:0x007ff619eab818>
/Users/xxx/project/gollum/gollum-lib/vendor/bundle/ruby/2.2.0/bundler/gems/rugged_adapter-e3fd3664642b/lib/rugged_adapter/git_layer_rugged.rb:282:in `pull'
/Users/xxx/project/gollum/gollum-lib/test/test_hook.rb:91:in `block (2 levels) in <top (required)>'

@bartkamphorst Would you help me?

hiroponz commented 8 years ago

I found the following in the rugged CHANGELOG.

Remote#clear_refspecs and Remote#save were removed without replacement.
hiroponz commented 8 years ago

FYI. https://github.com/libgit2/rugged/commit/559b09cc3e0659dcd0316050bdc44a8ec8e86aa1

bartkamphorst commented 8 years ago

Thanks @hiroponz . It looks like save is only called once, in the implementation of pull. Could you verify that removing that call does not break any functionality?

hiroponz commented 8 years ago

@bartkamphorst It seems that removing that call is OK, because it doesn't need to call Remote#save after Remote#fetch. Running the gollum-lib specs using rugged v.0.24 passed after removing that call.

bartkamphorst commented 8 years ago

Thanks. @dometto Any other thoughts or comments before I merge?

dometto commented 8 years ago

Go ahead :) we should check whether we can improve on the adapter with the improvements to rugged though (maybe there is no need for some of the custom methods in the adapter anymore)

bartkamphorst commented 8 years ago

Thanks @hiroponz ! :+1:

hiroponz commented 8 years ago

You are welcome :)

hiroponz commented 8 years ago

@dometto Would you release the gem of new version? It needs for this.

dometto commented 8 years ago

Released v0.4.2 just now.

hiroponz commented 8 years ago

Thanks!