Rebase wrapper #332

Merged
ezwiebel merged 10 commits from rebase-wrapper into master 2017-04-13 13:50:42 -05:00
3 changed files with 32 additions and 24 deletions
Showing only changes of commit a671e67ee8 - Show all commits

1
git.go
View File

@ -50,6 +50,7 @@ const (
ErrClassFilter ErrorClass = C.GITERR_FILTER
ErrClassRevert ErrorClass = C.GITERR_REVERT
ErrClassCallback ErrorClass = C.GITERR_CALLBACK
ErrClassRebase ErrorClass = C.GITERR_REBASE
)
type ErrorCode int

View File

@ -28,14 +28,14 @@ const (
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
// RebaseOperation describes a single instruction/operation to be performed during the rebase.
type RebaseOperation struct {
Type RebaseOperationType
ID *Oid
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
Id *Oid
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
Exec string
}
func newRebaseOperationFromC(c *C.git_rebase_operation) *RebaseOperation {
operation := &RebaseOperation{}
operation.Type = RebaseOperationType(c._type)
operation.ID = newOidFromC(&c.id)
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
operation.Id = newOidFromC(&c.id)
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
operation.Exec = C.GoString(c.exec)
return operation
@ -84,26 +84,26 @@ func (ro *RebaseOptions) toC() *C.git_rebase_options {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
version: C.uint(ro.Version),
quiet: C.int(ro.Quiet),
inmemory: C.int(ro.InMemory),
rewrite_notes_ref: rewriteNotesRefToC(ro.RewriteNotesRef),
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
rewrite_notes_ref: mapEmptyStringToNull(ro.RewriteNotesRef),
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
merge_options: *ro.MergeOptions.toC(),
checkout_options: *ro.CheckoutOptions.toC(),
}
}
func rewriteNotesRefToC(ref string) *C.char {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
func mapEmptyStringToNull(ref string) *C.char {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
if ref == "" {
return nil
}
return C.CString(ref)
}
carlosmn commented 2016-10-31 14:40:54 -05:00 (Migrated from github.com)
Review

This looks like it should have a much more generic name, since what it does is unrelated to note rewriting and it's simply mapping an empty string and NULL.

This looks like it should have a much more generic name, since what it does is unrelated to note rewriting and it's simply mapping an empty string and `NULL`.
ezwiebel commented 2016-10-31 18:58:29 -05:00 (Migrated from github.com)
Review

I understand your point. Renamed to mapEmptyStringToNull :p

I understand your point. Renamed to mapEmptyStringToNull :p
// Rebase object wrapper for C pointer
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
// Rebase is the struct representing a Rebase object.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
type Rebase struct {
ptr *C.git_rebase
}
//RebaseInit initializes a rebase operation to rebase the changes in branch relative to upstream onto another branch.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
func (r *Repository) RebaseInit(branch *AnnotatedCommit, upstream *AnnotatedCommit, onto *AnnotatedCommit, opts *RebaseOptions) (*Rebase, error) {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
// InitRebase initializes a rebase operation to rebase the changes in branch relative to upstream onto another branch.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
func (r *Repository) InitRebase(branch *AnnotatedCommit, upstream *AnnotatedCommit, onto *AnnotatedCommit, opts *RebaseOptions) (*Rebase, error) {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
runtime.LockOSThread()
defer runtime.UnlockOSThread()
@ -128,8 +128,8 @@ func (r *Repository) RebaseInit(branch *AnnotatedCommit, upstream *AnnotatedComm
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
return newRebaseFromC(ptr), nil
}
//RebaseOpen opens an existing rebase that was previously started by either an invocation of git_rebase_init or by another client.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
func (r *Repository) RebaseOpen(opts *RebaseOptions) (*Rebase, error) {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
// OpenRebase opens an existing rebase that was previously started by either an invocation of InitRebase or by another client.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
func (r *Repository) OpenRebase(opts *RebaseOptions) (*Rebase, error) {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
runtime.LockOSThread()
defer runtime.UnlockOSThread()
@ -149,9 +149,13 @@ func (rebase *Rebase) OperationAt(index uint) *RebaseOperation {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
}
// CurrentOperationIndex gets the index of the rebase operation that is currently being applied.
// If the first operation has not yet been applied then this returns -1 (C.GIT_REBASE_NO_OPERATION).
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
func (rebase *Rebase) CurrentOperationIndex() int {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
return int(C.git_rebase_operation_current(rebase.ptr))
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
// Returns an error if no rebase operation is currently applied.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
func (rebase *Rebase) CurrentOperationIndex() (uint, error) {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
operationIndex := int(C.git_rebase_operation_current(rebase.ptr))
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
if operationIndex == C.GIT_REBASE_NO_OPERATION {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
return 0, MakeGitError(C.GIT_REBASE_NO_OPERATION)
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
}
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
return uint(operationIndex), nil
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
}
// OperationCount gets the count of rebase operations that are to be applied.
@ -160,7 +164,7 @@ func (rebase *Rebase) OperationCount() uint {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
}
// Next performs the next rebase operation and returns the information about it.
// If the operation is one that applies a patch (which is any operation except GIT_REBASE_OPERATION_EXEC)
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
// If the operation is one that applies a patch (which is any operation except RebaseOperationExec)
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
// then the patch will be applied and the index and working directory will be updated with the changes.
// If there are conflicts, you will need to address those before committing the changes.
func (rebase *Rebase) Next() (*RebaseOperation, error) {
@ -177,7 +181,7 @@ func (rebase *Rebase) Next() (*RebaseOperation, error) {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
}
// Commit commits the current patch.
// You must have resolved any conflicts that were introduced during the patch application from the git_rebase_next invocation.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
// You must have resolved any conflicts that were introduced during the patch application from the Next() invocation.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
func (rebase *Rebase) Commit(ID *Oid, author, committer *Signature, message string) error {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
@ -192,6 +196,7 @@ func (rebase *Rebase) Commit(ID *Oid, author, committer *Signature, message stri
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
if err != nil {
return err
}
defer C.git_signature_free(committerSig)
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
cmsg := C.CString(message)
defer C.free(unsafe.Pointer(cmsg))
@ -229,7 +234,7 @@ func (rebase *Rebase) Abort() error {
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
return nil
}
//Free frees the Rebase object and underlying git_rebase C pointer.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
// Free frees the Rebase object.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
func (rebase *Rebase) Free() {
runtime.SetFinalizer(rebase, nil)
C.git_rebase_free(rebase.ptr)

carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:42:19 -05:00 (Migrated from github.com)
Review

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.

This describes the structure mechanically, but almost everything is a wrapper, if we're going to have a comment here, it should describe the object itself and/or what you can do with it.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:43:23 -05:00 (Migrated from github.com)
Review

We have prior art of naming git_repository_init() as InitRepository() since the namespacing in C and Go is bit different, so this should be called InitRebase().

We have prior art of naming `git_repository_init()` as `InitRepository()` since the namespacing in C and Go is bit different, so this should be called `InitRebase()`.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:44:06 -05:00 (Migrated from github.com)
Review

Like with the init function, this should be called OpenRebase(). The documentation shouldn't reference the C function git_rebase_init but our own.

Like with the init function, this should be called `OpenRebase()`. The documentation shouldn't reference the C function `git_rebase_init` but our own.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:45:41 -05:00 (Migrated from github.com)
Review

This return value does not match how Go reports errors. I would expect a uint to return the current operation and an error to return any errors.

This return value does not match how Go reports errors. I would expect a `uint` to return the current operation and an `error` to return any errors.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:46:46 -05:00 (Migrated from github.com)
Review

There is no GIT_REBASE_OPERATION_EXEC in git2go, it's RebaseOperationExec.

There is no `GIT_REBASE_OPERATION_EXEC` in git2go, it's `RebaseOperationExec`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:47:37 -05:00 (Migrated from github.com)
Review

There is no git_rebase_next in git2go, we have Next().

There is no `git_rebase_next` in git2go, we have `Next()`.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:50:26 -05:00 (Migrated from github.com)
Review

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.

Missing space. The pointer isn't public so let's not mention it. "unmanaged resources" covers whatever we decide to put in there.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
carlosmn commented 2016-10-31 14:54:32 -05:00 (Migrated from github.com)
Review

The other public id fields are named Id.

The other public id fields are named `Id`.
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:57:15 -05:00 (Migrated from github.com)
Review

Ok, I followed go convention suggested by my IDE plugin

Ok, I followed go convention suggested by my IDE plugin
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:38 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:42 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 18:58:49 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:02 -05:00 (Migrated from github.com)
Review

Done.
I also added ErrClassRebase in git.go
Hope this is correct

Done. I also added `ErrClassRebase` in `git.go` Hope this is correct
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:09 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done
ezwiebel commented 2016-10-31 19:01:27 -05:00 (Migrated from github.com)
Review

Done

Done

View File

@ -111,7 +111,7 @@ func TestRebaseNoConflicts(t *testing.T) {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
seedTestRepo(t, repo)
// Try to open existing rebase
oRebase, err := repo.RebaseOpen(nil)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
oRebase, err := repo.OpenRebase(nil)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
if err == nil {
t.Fatal("Did not expect to find a rebase in progress")
}
@ -132,7 +132,7 @@ func TestRebaseNoConflicts(t *testing.T) {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
defer rebase.Free()
// Open existing rebase
oRebase, err = repo.RebaseOpen(nil)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
oRebase, err = repo.OpenRebase(nil)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
checkFatal(t, err)
defer oRebase.Free()
if oRebase == nil {
@ -144,7 +144,7 @@ func TestRebaseNoConflicts(t *testing.T) {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
checkFatal(t, err)
// Check no more rebase is in progress
oRebase, err = repo.RebaseOpen(nil)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
oRebase, err = repo.OpenRebase(nil)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
if err == nil {
t.Fatal("Did not expect to find a rebase in progress")
}
@ -198,13 +198,14 @@ func performRebaseOnto(repo *Repository, branch string) (*Rebase, error) {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
defer onto.Free()
// Init rebase
rebase, err := repo.RebaseInit(nil, nil, onto, nil)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
rebase, err := repo.InitRebase(nil, nil, onto, nil)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
if err != nil {
return nil, err
}
// Check no operation has been started yet
if rebase.CurrentOperationIndex() != -1 { // -1 == GIT_REBASE_NO_OPERATION
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
rebaseOperationIndex, err := rebase.CurrentOperationIndex()
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
if err == nil {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
return nil, errors.New("No operation should have been started yet")
}
@ -217,7 +218,8 @@ func performRebaseOnto(repo *Repository, branch string) (*Rebase, error) {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
}
// Check operation index is correct
if rebase.CurrentOperationIndex() != op {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
rebaseOperationIndex, err = rebase.CurrentOperationIndex()
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
if int(rebaseOperationIndex) != op {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
return nil, errors.New("Bad operation index")
}
if !operationsAreEqual(rebase.OperationAt(uint(op)), operation) {
@ -225,14 +227,14 @@ func performRebaseOnto(repo *Repository, branch string) (*Rebase, error) {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
}
// Get current rebase operation created commit
commit, err := repo.LookupCommit(operation.ID)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
commit, err := repo.LookupCommit(operation.Id)
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
if err != nil {
return nil, err
}
defer commit.Free()
// Apply commit
err = rebase.Commit(operation.ID, signature(), signature(), commit.Message())
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
err = rebase.Commit(operation.Id, signature(), signature(), commit.Message())
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
if err != nil {
return nil, err
}
@ -242,7 +244,7 @@ func performRebaseOnto(repo *Repository, branch string) (*Rebase, error) {
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
}
func operationsAreEqual(l, r *RebaseOperation) bool {
return l.Exec == r.Exec && l.Type == r.Type && l.ID.String() == r.ID.String()
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
return l.Exec == r.Exec && l.Type == r.Type && l.Id.String() == r.Id.String()
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
}
func createBranch(repo *Repository, branch string) error {

carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
carlosmn commented 2016-10-31 14:56:01 -05:00 (Migrated from github.com)
Review

Don't hard-code 1 here, the point of the version field is to increase it.

Don't hard-code 1 here, the point of the version field is to increase it.
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
ezwiebel commented 2016-10-31 19:03:04 -05:00 (Migrated from github.com)
Review

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a #define or a constant that was used as default version number.
The test doesn't have great value though

Not sure how to handle this. It's a test that checks the default version returned by DefaultRebaseOptions. After a look into C code, I thought it was a `#define` or a constant that was used as default version number. The test doesn't have great value though
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
carlosmn commented 2016-11-13 12:33:45 -06:00 (Migrated from github.com)
Review

The version it needs to return in the version that is specified by the libgit2 headers in the #define. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.

The version it needs to return in the version that is specified by the libgit2 headers in the `#define`. That's what the compiler is building and that's what it will use in order to figure out the size of the struct, which has to match what we're telling libgit2 we have.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.
ezwiebel commented 2016-11-14 02:13:44 -06:00 (Migrated from github.com)
Review

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.

Thanks for that. However I'm not sure how I should do as the cgo feature seems to be forbidden in tests... I will just drop this very low value test I think.