Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

plumbing: object, commit.Parent() method #534

Merged
merged 2 commits into from
Nov 20, 2017

Conversation

josharian
Copy link
Contributor

First parents are somewhat special in git.
There's even a --first-parent flag to 'git log'.

Add a helper method to look them up.
This avoids boilerplate and spares the client from
having to arrange for a handle to the Storer,
which is stored in the unexported field Commit.s.

First parents are somewhat special in git.
There's even a --first-parent flag to 'git log'.

Add a helper method to look them up.
This avoids boilerplate and spares the client from
having to arrange for a handle to the Storer,
which is stored in the unexported field Commit.s.
var ErrNoParents = errors.New("commit has no parents")

// FirstParent returns the first parent of c.
func (c *Commit) FirstParent() (*Commit, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you create create test or this?

@mcuadros
Copy link
Contributor

@ajnavarro @orirawlings what do you thing about this?

@orirawlings
Copy link
Contributor

I agree it needs a test, but I think it can be useful.

Perhaps it would be more useful to generalize further:

// Parent returns the ith parent of c. 
func (c *Commit) Parent(i int) (*Commit, error) {

@ajnavarro
Copy link
Contributor

FirstParent() appears good to me, it has some use cases. Into the method documentation I would add what happens if the commit does not have parents.

@mcuadros
Copy link
Contributor

@orirawlings this is the signature from "git2go" and I must to say that for me wasn't clear that the first commit has any special meaning. But it's because doesn't have it, the order of the parents is something relevant when you are flattering the tree (like in git log).

So yes, I prefer the signature

func (c *Commit) Parent(i int) (*Commit, error) {

@joshbetz can you update the PR with the request? Thanks!

@joshbetz
Copy link
Contributor

I think you mean @josharian :)

@mcuadros
Copy link
Contributor

Yup, sorry.

@josharian
Copy link
Contributor Author

Thanks for the review. Will add a test and update docs a bit later; distracted by other matters at the moment.

@mcuadros
Copy link
Contributor

@josharian ping

@mcuadros mcuadros changed the title plumbing/object: add Commit.FirstParent "plumbing: object, commit.Parent() method" Nov 20, 2017
Signed-off-by: Máximo Cuadros <mcuadros@gmail.com>
@mcuadros mcuadros changed the title "plumbing: object, commit.Parent() method" plumbing: object, commit.Parent() method Nov 20, 2017
@mcuadros mcuadros merged commit 4cbf210 into src-d:master Nov 20, 2017
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants