-
Notifications
You must be signed in to change notification settings - Fork 14
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
User defined commit message templates #40
Conversation
var messageTemplate, messageWithTicketTemplate string | ||
if c.MessageTemplate == nil { | ||
messageTemplate = defaultMessageTemplate | ||
} else { | ||
messageTemplate, err = config.ConvertTemplate(*c.MessageTemplate) | ||
if err != nil { | ||
log.Error("Error converting message template", "error", err) | ||
messageTemplate = defaultMessageTemplate | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might ne essier to reader
var messageTemplate, messageWithTicketTemplate string | |
if c.MessageTemplate == nil { | |
messageTemplate = defaultMessageTemplate | |
} else { | |
messageTemplate, err = config.ConvertTemplate(*c.MessageTemplate) | |
if err != nil { | |
log.Error("Error converting message template", "error", err) | |
messageTemplate = defaultMessageTemplate | |
} | |
} | |
var messageWithTicketTemplate string | |
messageTemplate = defaultMessageTemplate | |
if c.MessageTemplate != nil { | |
messageTemplate, err = config.ConvertTemplate(*c.MessageTemplate) | |
if err != nil { | |
log.Error("Error converting message template", "error", err) | |
messageTemplate = defaultMessageTemplate | |
} | |
} |
if c.MessageWithTicketTemplate == nil { | ||
messageWithTicketTemplate = defaultMessageWithTicketTemplate | ||
} else { | ||
messageWithTicketTemplate, err = config.ConvertTemplate(*c.MessageWithTicketTemplate) | ||
if err != nil { | ||
log.Error("Error converting message with ticket template", "error", err) | ||
messageWithTicketTemplate = defaultMessageWithTicketTemplate | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could apply the same pattern I described here
@@ -85,7 +86,7 @@ func main() { | |||
fail("Could not change directory: %s", err) | |||
} | |||
|
|||
prefixes, coauthors, boards, showIntro, commitTitleCharLimit, err := loadConfig(AFS) | |||
prefixes, coauthors, boards, showIntro, commitTitleCharLimit, messageTemplate, messageWithTicketTemplate, err := loadConfig(AFS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could consider returning a struct, there are way too much returned values
if !strings.Contains(t, "@type") || !strings.Contains(t, "@message") { | ||
return t, fmt.Errorf("template must contain @type and @message") | ||
} | ||
t = strings.Replace(t, ":", "{{if .IsBreakingChange}}!{{end}}:", 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your template system might be "hacked" if someone provide a `{{ whatever }}`` syntax
Maybe you could forbid the presence of {{
in the template
@ccoVeille thanks for the review! unfortunately I had already merged this code, but I've created a new PR addressing the issues raised here #41, would be great if you could have a look when you have some time! |
This PR adds two new config options (
messageTemplate
andmessageWithTicketTemplate
), which allow the user to provide their own custom commit message templates.If no templates are provided, defaults which are the same as before are used to ensure backwards compatibility for all users.