Reflog support #730

2021-01-18T11:05:06-06:00

pokstad commented

2021-01-18 11:05:06 -06:00

(Migrated from github.com)

Adds support and tests for all reflog functions listed on https://libgit2.org/libgit2/#HEAD/group/reflog.

Closes #467

Adds support and tests for all reflog functions listed on https://libgit2.org/libgit2/#HEAD/group/reflog. Closes #467

lhchavez (Migrated from github.com) reviewed 2021-01-27 21:03:18 -06:00

lhchavez (Migrated from github.com) left a comment

sorry for the delay! for some reason i saw this, accidentally marked as read and completely forgot about it until today u_u

reflog.go

													
				@ -0,0 +26,4 @@

					return l

				}

				func (repo *Repository) ReadReflog(name string) (*Reflog, error) {

lhchavez (Migrated from github.com) commented

2021-01-27 20:49:53 -06:00

in order for these functions to be more easy to find in the docs, what do you think about modelling this similar to the other *Collection structs in https://godoc.org/github.com/libgit2/git2go#Repository? (i.e. there would be an Reflogs ReflogCollection). That way folks can do repo.Reflogs.Read("name") and repo.Reflogs.Delete("name").

also, i know we have not been super diligent about this, but could all public methods being introduced have a docstring? the godocs need a bit of love ^^;;

in order for these functions to be more easy to find in the docs, what do you think about modelling this similar to the other *Collection structs in https://godoc.org/github.com/libgit2/git2go#Repository? (i.e. there would be an `Reflogs ReflogCollection`). That way folks can do `repo.Reflogs.Read("name")` and `repo.Reflogs.Delete("name")`. also, i know we have not been super diligent about this, but could all public methods being introduced have a docstring? the godocs need a bit of love ^^;;

reflog.go

													
				@ -0,0 +117,4 @@

					}

				}

				func (l *Reflog) EntryByIndex(index uint) *ReflogEntry {

lhchavez (Migrated from github.com) commented

2021-01-27 20:57:35 -06:00

func (l *Reflog) EntryByIndex(index uint64) *ReflogEntry {

would it be possible to make this the widest size for any architecture that we support today? supporting more than 2^32 entries might be silly, but i'd rather be safe than sorry.

same in all the other places that use size_t in the C api.

```suggestion func (l *Reflog) EntryByIndex(index uint64) *ReflogEntry { ``` would it be possible to make this the widest size for any architecture that we support today? supporting more than 2^32 entries might be silly, but i'd rather be safe than sorry. same in all the other places that use `size_t` in the C api.

reflog.go

													
				@ -0,0 +136,4 @@

					runtime.LockOSThread()

					defer runtime.UnlockOSThread()

					var rewriteHistoryInt int

lhchavez (Migrated from github.com) commented

2021-01-27 21:00:26 -06:00

	var rewriteHistoryInt C.int

might as well define it as C.int from the get-go to avoid the cast in L144.

```suggestion var rewriteHistoryInt C.int ``` might as well define it as `C.int` from the get-go to avoid the cast in L144.

reflog.go

													
				@ -0,0 +163,4 @@

					cMsg := C.CString(message)

					defer C.free(unsafe.Pointer(cMsg))

					C.git_reflog_append(l.ptr, oid.toC(), cSignature, cMsg)