Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Response to code review comments: refactors getting author logic
Browse files Browse the repository at this point in the history
  • Loading branch information
Tamara Kaufler committed Sep 13, 2017
1 parent c7dd6ac commit 259a03c
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 109 deletions.
42 changes: 17 additions & 25 deletions cmd/fluxctl/args.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,36 +19,27 @@ func parseServiceOption(s string) (update.ServiceSpec, error) {
}

func AddCauseFlags(cmd *cobra.Command, opts *update.Cause) {
username := getCommitAuthor()
authorInfo := getUserGitconfig()
username := getCommitAuthor(authorInfo)

cmd.Flags().StringVarP(&opts.Message, "message", "m", "", "attach a message to the update")
cmd.Flags().StringVar(&opts.User, "user", username, "override the user reported as initiating the update")
}

func getCommitAuthor() string {
authorInfo := getUserGitconfig()
if userName, ok := authorInfo["user.name"]; ok {
switch userName {
case "":
return getUserEmail(authorInfo)
default:
if userEmail, ok := authorInfo["user.email"]; ok {
if userEmail == "" {
return userName
}
return fmt.Sprintf("%s <%s>", userName, userEmail)
}
}
}
return getUserEmail(authorInfo)
}
func getCommitAuthor(authorInfo map[string]string) string {
userName := authorInfo["user.name"]
userEmail := authorInfo["user.email"]

func getUserEmail(gitconfig map[string]string) string {
if userEmail, ok := gitconfig["user.email"]; ok {
switch userName {
case "":
if userEmail != "" {
return userEmail
}
default:
if userEmail == "" {
return ""
return userName
}
return userEmail
return fmt.Sprintf("%s <%s>", userName, userEmail)
}
return ""
}
Expand All @@ -71,12 +62,13 @@ func userGitconfigMap(s string) map[string]string {
c := make(map[string]string)
lines := splitList(s)
for _, l := range lines {
if l == "" {
continue
}
prop := strings.SplitN(l, "=", 2)
p := strings.TrimSpace(prop[0])
v := strings.TrimSpace(prop[1])
if p == "user.name" || p == "user.email" {
c[p] = v
}
c[p] = v
}
return c
}
Expand Down
189 changes: 111 additions & 78 deletions cmd/fluxctl/args_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,125 +12,158 @@ func TestUserGitconfigMap_EmptyString(t *testing.T) {
t.Fatal("expected no content")
}
}
func TestUserGitconfigMap_StringWithoutKeys(t *testing.T) {
d := `
push.default=simple
func TestUserGitconfigMap(t *testing.T) {
d := `push.default=simple
merge.conflictstyle=diff3
pull.ff=only
core.repositoryformatversion=0
core.filemode=true
core.bare=false
`
core.bare=false`

userGitconfigInfo := userGitconfigMap(d)
if len(userGitconfigInfo) != 0 {
if len(userGitconfigInfo) != 6 {
t.Fatal("expected no content")
}
}
func TestUserGitconfigMap_BothKeys(t *testing.T) {
func TestUserGitconfigMap_WithEmptyLines(t *testing.T) {
d := `
user.email=jd@w.ws
user.name=Jane Doe
push.default=simple
merge.conflictstyle=diff3
pull.ff=only
core.repositoryformatversion=0
core.filemode=true
core.bare=false
`
expected := make(map[string]string)
expected["user.email"] = "jd@w.ws"
expected["user.name"] = "Jane Doe"
userGitconfigInfo := userGitconfigMap(d)

if len(userGitconfigInfo) != 2 {
t.Fatal("expected map with 2 keys")
}
if !reflect.DeepEqual(userGitconfigInfo, expected) {
t.Fatal("result does not match expected structure")
}
}
func TestUserGitconfigMap_OnlyNameKey(t *testing.T) {
d := `
user.name=Jane Doe
push.default=simple
merge.conflictstyle=diff3
pull.ff=only
core.repositoryformatversion=0
core.filemode=true
core.bare=false
`
expected := make(map[string]string)
expected["user.name"] = "Jane Doe"
expected := map[string]string{
"user.name": "Jane Doe",
"push.default": "simple",
"merge.conflictstyle": "diff3",
"pull.ff": "only",
"core.repositoryformatversion": "0",
"core.filemode": "true",
"core.bare": "false",
}
userGitconfigInfo := userGitconfigMap(d)

if len(userGitconfigInfo) != 1 {
if len(userGitconfigInfo) != 7 {
t.Fatal("expected map with 1 key")
}
if !reflect.DeepEqual(userGitconfigInfo, expected) {
t.Fatal("result does not match expected structure")
}
}
func TestUserGitconfigMap_OnlyNameKeyAndEmpty(t *testing.T) {
func TestUserGitconfigMap_WithNoKeys(t *testing.T) {
d := `
user.name=
push.default=simple
merge.conflictstyle=diff3
pull.ff=only
core.repositoryformatversion=0
core.filemode=true
core.bare=false
`
expected := make(map[string]string)
expected["user.name"] = ""

userGitconfigInfo := userGitconfigMap(d)
if len(userGitconfigInfo) != 1 {
if len(userGitconfigInfo) != 0 {
t.Fatal("expected map with one key")
}
if !reflect.DeepEqual(userGitconfigInfo, expected) {
t.Fatal("result does not match expected structure")
}
}
func TestUserGitconfigMap_OnlyEmailKey(t *testing.T) {
d := `
user.email=jd@a.b
push.default=simple
merge.conflictstyle=diff3
pull.ff=only
core.repositoryformatversion=0
core.filemode=true
core.bare=false
`
expected := make(map[string]string)
expected["user.email"] = "jd@a.b"
userGitconfigInfo := userGitconfigMap(d)

if len(userGitconfigInfo) != 1 {
t.Fatal("expected map with 1 key")
func TestGetCommitAuthor_BothNameAndEmail(t *testing.T) {
input := map[string]string{
"user.name": "Jane Doe",
"user.email": "jd@j.d",
"push.default": "simple",
"merge.conflictstyle": "diff3",
"pull.ff": "only",
"core.repositoryformatversion": "0",
"core.filemode": "true",
"core.bare": "false",
}
if !reflect.DeepEqual(userGitconfigInfo, expected) {
t.Fatal("result does not match expected structure")
expected := "Jane Doe <jd@j.d>"
author := getCommitAuthor(input)
if author != expected {
t.Fatal("author did not match expected value")
}
}
func TestGetCommitAuthor_OnlyName(t *testing.T) {
input := map[string]string{
"user.name": "Jane Doe",
"push.default": "simple",
"merge.conflictstyle": "diff3",
"pull.ff": "only",
"core.repositoryformatversion": "0",
"core.filemode": "true",
"core.bare": "false",
}
expected := "Jane Doe"
author := getCommitAuthor(input)
if author != expected {
t.Fatal("author did not match expected value")
}
}
func TestUserGitconfigMap_OnlyEmailKeyAndEmpty(t *testing.T) {
d := `
user.email=
push.default=simple
merge.conflictstyle=diff3
pull.ff=only
core.repositoryformatversion=0
core.filemode=true
core.bare=false
`
expected := make(map[string]string)
expected["user.email"] = ""

userGitconfigInfo := userGitconfigMap(d)
if len(userGitconfigInfo) != 1 {
t.Fatal("expected map with one key")
func TestGetCommitAuthor_OnlyEmail(t *testing.T) {
input := map[string]string{
"user.email": "jd@j.d",
"push.default": "simple",
"merge.conflictstyle": "diff3",
"pull.ff": "only",
"core.repositoryformatversion": "0",
"core.filemode": "true",
"core.bare": "false",
}
if !reflect.DeepEqual(userGitconfigInfo, expected) {
t.Fatal("result does not match expected structure")
expected := "jd@j.d"
author := getCommitAuthor(input)
if author != expected {
t.Fatal("author did not match expected value")
}
}
func TestGetCommitAuthor_NoNameNoEmail(t *testing.T) {
input := map[string]string{
"push.default": "simple",
"merge.conflictstyle": "diff3",
"pull.ff": "only",
"core.repositoryformatversion": "0",
"core.filemode": "true",
"core.bare": "false",
}
expected := ""
author := getCommitAuthor(input)
if author != expected {
t.Fatal("author did not match expected value")
}
}
func TestGetCommitAuthor_NameAndEmptyEmail(t *testing.T) {
input := map[string]string{
"user.name": "Jane Doe",
"user.email": "",
"push.default": "simple",
"merge.conflictstyle": "diff3",
"pull.ff": "only",
"core.repositoryformatversion": "0",
"core.filemode": "true",
"core.bare": "false",
}
expected := "Jane Doe"
author := getCommitAuthor(input)
if author != expected {
t.Fatal("author did not match expected value")
}
}
func TestGetCommitAuthor_EmailAndEmptyName(t *testing.T) {
input := map[string]string{
"user.name": "",
"user.email": "jd@j.d",
"push.default": "simple",
"merge.conflictstyle": "diff3",
"pull.ff": "only",
"core.repositoryformatversion": "0",
"core.filemode": "true",
"core.bare": "false",
}
expected := "jd@j.d"
author := getCommitAuthor(input)
if author != expected {
t.Fatal("author did not match expected value")
}
}
6 changes: 0 additions & 6 deletions git/operations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,6 @@ func clone(ctx context.Context, workingDir string, keyRing ssh.KeyRing, repoURL,
}

func commit(ctx context.Context, workingDir string, commitAction *CommitAction) error {
// The user in the update.Cause.User must be a valid github user, otherwise the commit fails
// Hardcoding of the user to ignore is not ideal. If there is only one to ignore, ie test@test,
// then this is just about ok. Otherwise there could be config file with stored users to ignore etc.
// Currently the committer is hardcoded to flux-bot for user requested actions.
// TODO (potentially): allow the users to set the committer, ie have an additional
// update.Cause.Committer field.
commitAuthor := commitAction.Author
if commitAuthor != "" {
if err := execGitCmd(ctx,
Expand Down

0 comments on commit 259a03c

Please sign in to comment.