Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .helm-staging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ providers:
app_id: 17877
private_key: /local/lookout/private-key.pem
secretName: lookout-staging-github-key
comment_footer: '_If you have feedback about this comment, please, [tell us](%s)._'
comment_footer: "_{{if .Feedback}}If you have feedback about this comment made by the analyzer {{.Name}}, please, [tell us]({{.Feedback}}){{else}}Comment made by the analyzer {{.Name}}{{end}}._"
installation_sync_interval: 5m

analyzers:
Expand Down
2 changes: 1 addition & 1 deletion cmd/lookoutd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ func (c *queueConsumerCommand) initPoster(conf Config) (lookout.Poster, error) {

switch c.Provider {
case github.Provider:
return github.NewPoster(c.pool, conf.Providers.Github), nil
return github.NewPoster(c.pool, conf.Providers.Github)
case json.Provider:
return json.NewPoster(os.Stdout), nil
default:
Expand Down
2 changes: 1 addition & 1 deletion config.yml.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ analyzers:

providers:
github:
comment_footer: '_If you have feedback about this comment, please, [tell us](%s)._'
comment_footer: "_{{if .Feedback}}If you have feedback about this comment made by the analyzer {{.Name}}, please, [tell us]({{.Feedback}}){{else}}Comment made by the analyzer {{.Name}}{{end}}._"
# The minimum watch interval to discover new pull requests and push events
watch_min_interval: 2s
# Authorization with GitHub App
Expand Down
34 changes: 24 additions & 10 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ The `providers.github` key configures how **source{d} Lookout** will connect wit
```yaml
providers:
github:
comment_footer: "_If you have feedback about this comment, please, [tell us](%s)._"
comment_footer: "_Comment made by '{{.Name}}'{{with .Feedback}}, [tell us]({{.}}){{end}}._"
# app_id: 1234
# private_key: ./key.pem
# installation_sync_interval: 1h
# watch_min_interval: 2s
```

`comment_footer` key defines a format-string that will be used for custom messages for every message posted on GitHub; see how to [add a custom message to the posted comments](#add-a-custom-message-to-the-posted-comments)
`comment_footer` key defines the [go template](https://golang.org/pkg/text/template) that will be used for custom messages for every message posted on GitHub; see how to [add a custom message to the posted comments](#add-a-custom-message-to-the-posted-comments)

### Authentication with GitHub

Expand Down Expand Up @@ -141,17 +141,31 @@ analyzers:

### Add a Custom Message to the Posted Comments

You can configure **source{d} Lookout** to add a custom message to every comment that each analyzer returns. This custom message will be created following the rule:
```
sprintf(providers.github.comment_footer, feedback)
```
If any of those two keys are not defined, the custom message won't be added.
You can configure **source{d} Lookout** to add a custom message to every comment that each analyzer returns. This custom message will be created from the template defined by `providers.github.comment_footer`, using the configuration set for each analyzer.

Example:
```text
"_If you have feedback about this comment, please, [tell us](%s)._"
If the template (`providers.github.comment_footer`) is empty, or the analyzer configuration does not define any of the values that the template requires, the custom message won't be added.

For example, for this configuration, each analyzer needs to define `name` and `settings.email`:

```yaml
providers:
github:
comment_footer: "Comment made by analyzer {{.Name}}, [email me]({{.Settings.email}})."

analyzers:
- name: Fancy Analyzer
addr: ipv4://localhost:9930
settings:
email: admin@example.org
- name: Awesome Analyzer
addr: ipv4://localhost:9931
```

Comments from `Fancy Analyzer` will have this footer appended:
>_Comment made by analyzer Fancy Analyzer, [email me](admin@example.org)._

but comments from `Awesome Analyzer` wont have a footer message because in its configuration it's missing the `settings.email` value.


## Timeouts

Expand Down
30 changes: 18 additions & 12 deletions provider/github/poster.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"reflect"
"strings"
"text/template"

"github.com/src-d/lookout"
"github.com/src-d/lookout/util/ctxlog"
Expand Down Expand Up @@ -32,18 +33,27 @@ const (

// Poster posts comments as Pull Request Reviews.
type Poster struct {
pool *ClientPool
conf ProviderConfig
pool *ClientPool
conf ProviderConfig
footerTemplate *template.Template
}

var _ lookout.Poster = &Poster{}

// NewPoster creates a new poster for the GitHub API.
func NewPoster(pool *ClientPool, conf ProviderConfig) *Poster {
return &Poster{
pool: pool,
conf: conf,
func NewPoster(pool *ClientPool, conf ProviderConfig) (*Poster, error) {
tpl, err := newFooterTemplate(conf.CommentFooter)
if ErrEmptyTemplate.Is(err) {
log.DefaultLogger.Warningf("no footer template being used: %s", err)
} else if err != nil {
return nil, err
}

return &Poster{
pool: pool,
conf: conf,
footerTemplate: tpl,
}, nil
}

// Post posts comments as a Pull Request Review.
Expand Down Expand Up @@ -157,15 +167,11 @@ func (p *Poster) createReviewRequest(
}

var bodyComments []string
tmpl := p.conf.CommentFooter

for _, aComments := range aCommentsList {
ctx, _ := ctxlog.WithLogFields(ctx, log.Fields{
"analyzer": aComments.Config.Name,
})

url := aComments.Config.Feedback

forBody, ghComments := convertComments(ctx, aComments.Comments, dl)

if len(postedComments) > 0 {
Expand All @@ -175,13 +181,13 @@ func (p *Poster) createReviewRequest(
ghComments = mergeComments(ghComments)

for i, c := range ghComments {
body := addFootnote(c.GetBody(), tmpl, url)
body := addFootnote(ctx, c.GetBody(), p.footerTemplate, &aComments.Config)
ghComments[i].Body = &body
}

bodyComments = append(
bodyComments,
addFootnote(strings.Join(forBody, "\n\n"), tmpl, url),
addFootnote(ctx, strings.Join(forBody, "\n\n"), p.footerTemplate, &aComments.Config),
)
req.Comments = append(req.Comments, ghComments...)
}
Expand Down
26 changes: 22 additions & 4 deletions provider/github/poster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,10 @@ func (s *PosterTestSuite) TestPostFooter() {
aComments := mockAnalyzerComments
aComments[0].Config.Feedback = "https://foo.bar/feedback"

footerTpl, _ := newFooterTemplate("To post feedback go to {{.Feedback}}")
p := &Poster{
pool: s.pool,
conf: ProviderConfig{
CommentFooter: "To post feedback go to %s",
},
pool: s.pool,
footerTemplate: footerTpl,
}
err := p.Post(context.Background(), mockEvent, aComments, false)
s.NoError(err)
Expand Down Expand Up @@ -526,3 +525,22 @@ func intptr(v int) *int {
func int64ptr(v int64) *int64 {
return &v
}

func (s *PosterTestSuite) TestCouldNotParseFooterTemplate() {
emptyTemplateRaw := ""
posterWithEmptyTemplate, err := NewPoster(nil, ProviderConfig{CommentFooter: emptyTemplateRaw})
s.Nil(err, "NewPoster must return no error when parsing an empty template")
emptyTemplate := posterWithEmptyTemplate.footerTemplate
commentsEmptyTemplate := addFootnote(context.TODO(), "comments", emptyTemplate, nil)
s.Equal("comments", commentsEmptyTemplate)

oldTemplateRaw := "Old template %s"
posterWithOldTemplate, err := NewPoster(nil, ProviderConfig{CommentFooter: oldTemplateRaw})
s.Nil(posterWithOldTemplate, "NewPoster must fail when parsing an old template config")
s.True(ErrOldTemplate.Is(err), "Error should be 'ErrOldTemplate'")

wrongTemplateeRaw := "Old template {{{parseerror"
posterWithWrongTemplate, err := NewPoster(nil, ProviderConfig{CommentFooter: wrongTemplateeRaw})
s.Nil(posterWithWrongTemplate, "NewPoster must fail when parsing a wrong template config")
s.True(ErrParseTemplate.Is(err), "Error should be 'ErrParseTemplate'")
}
58 changes: 53 additions & 5 deletions provider/github/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ package github

import (
"context"
"fmt"
"sort"
"strings"
"text/template"
"time"

"github.com/google/go-github/github"
"github.com/src-d/lookout"
"github.com/src-d/lookout/util/ctxlog"
errors "gopkg.in/src-d/go-errors.v1"
log "gopkg.in/src-d/go-log.v1"
)

Expand All @@ -29,6 +30,13 @@ const commentsSeparator = "\n<!-- lookout comment separator -->\n---\n"
// comment can contain footer with link to the analyzer
const footnoteSeparator = "\n<!-- lookout footnote separator -->\n"

var (
ErrEmptyTemplate = errors.NewKind("empty footer template")
ErrOldTemplate = errors.NewKind("old footer template: '%%s' placeholder is no longer supported: '%s'")
ErrParseTemplate = errors.NewKind("error parsing footer template: %s")
ErrTemplateError = errors.NewKind("error generating the footer: %s")
)

// createReview creates pull request review on github using multiple http calls
// in case of too many comments
func createReview(
Expand Down Expand Up @@ -207,13 +215,53 @@ func splitReviewRequest(review *github.PullRequestReviewRequest, n int) []*githu
return result
}

func newFooterTemplate(tpl string) (*template.Template, error) {
if tpl == "" {
return nil, ErrEmptyTemplate.New()
}

if strings.Index(tpl, "%s") >= 0 {
return nil, ErrOldTemplate.New(tpl)
}

template, err := template.New("footer").Parse(tpl)
if err != nil {
return nil, ErrParseTemplate.New(err)
}

return template.Option("missingkey=error"), nil
}

// addFootnote adds footnote link to text of a comment
func addFootnote(text, tmpl, url string) string {
if text == "" || tmpl == "" || url == "" {
return text
func addFootnote(
ctx context.Context,
comment string, tmpl *template.Template, analyzerConf *lookout.AnalyzerConfig,
) string {
if comment == "" || tmpl == nil {
return comment
}

footer, err := getFootnote(tmpl, analyzerConf)
if err != nil {
ctxlog.Get(ctx).Warningf("footer could not be generated: %s", err)
return comment
}

return comment + footer
}

func getFootnote(tmpl *template.Template, analyzerConf *lookout.AnalyzerConfig) (string, error) {
var footer strings.Builder
if err := tmpl.Execute(&footer, analyzerConf); err != nil {
return "", ErrTemplateError.New(err)
}

footerTxt := footer.String()
if footerTxt == "" {
return "", nil
}

return text + footnoteSeparator + fmt.Sprintf(tmpl, url)
return footnoteSeparator + footer.String(), nil
}

// removeFootnote removes footnote and returns only text of a comment
Expand Down
9 changes: 9 additions & 0 deletions provider/github/review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,3 +225,12 @@ func TestConvertCommentsWrongFile(t *testing.T) {
Body: strptr("Line comment"),
}}, ghComments)
}

func TestCouldNotExecuteFooterTemplate(t *testing.T) {
require := require.New(t)

unkonwnDataTemplate, err := newFooterTemplate("Old template {{.UnknownData}}")
require.Nil(err)
commentsWrongTemplate := addFootnote(context.TODO(), "comments", unkonwnDataTemplate, nil)
require.Equal("comments", commentsWrongTemplate)
}