Skip to content

feat: include commit options for repo.Commit and repo.InitAndCommit #173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ChBLA
Copy link
Contributor

@ChBLA ChBLA commented Jul 10, 2025

This is a breaking change for the git package!

This allows the consumer to provide commit options. This is useful for setting the Author or Committer manually, instead of letting go-git automatically detect it. Since go-git/go-git#110, if you do not have author set in $HOME/.gitconfig, repo.Commit fails.

@ChBLA ChBLA requested a review from a team as a code owner July 10, 2025 09:30
@ChBLA
Copy link
Contributor Author

ChBLA commented Jul 10, 2025

If it is not desired to break the API for Commit and InitAndCommit, another function SetAuthor could be used which would save the author to the gitRepository struct.

@KimNorgaard
Copy link
Contributor

KimNorgaard commented Aug 7, 2025

If it is not desired to break the API for Commit and InitAndCommit, another function SetAuthor could be used which would save the author to the gitRepository struct.

If you want to keep it backward compatible I think we could use variadic options (or function options):

	InitAndCommit(dir string, url string, cfg *config.Config, opts ...*git.CommitOptions)

Then use opts[0] if present.

or more elaborate:

type GitOptions struct {
	CommitOptions *git.CommitOptions
}

type GitOption func(*GitOptions)

func WithCommitOptions(o *git.CommitOptions) GitOption {
	return func(go *GitOptions) {
		go.CommitOptios = o
	}
}

func InitAndCommit(dir string, url string, cfg *config.Config, opts ...GitOption) {
	gitoptions := &GitOptions{}
	for _, opt := range opts {
		opt(gitOptions)
	}
}

or something like that :)

I'm partial to the latter solution as it is more intuitive, doesn't pollute the signature, doesn't require knowing which index in the options you are looking at and is easier to extend.

Copy link
Contributor

@KimNorgaard KimNorgaard left a comment

Choose a reason for hiding this comment

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

See comment

@ChBLA ChBLA requested a review from KimNorgaard August 7, 2025 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants