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

Add dingtalk webhook #2777

Merged
merged 7 commits into from Nov 21, 2017

Conversation

5 participants
@lunny
Member

lunny commented Oct 25, 2017

As title.

@lunny lunny added the kind/feature label Oct 25, 2017

@lunny lunny added this to the 1.4.0 milestone Oct 25, 2017

@lunny lunny force-pushed the lunny:lunny/webhook_ding_talk branch from 024291a to f33e2e3 Oct 25, 2017

@ethantkoenig

ethantkoenig suggested changes Oct 25, 2017 edited

Tests would be nice

// for each commit, generate attachment text
for i, commit := range p.Commits {
text += fmt.Sprintf("[%s](%s) %s - %s", commit.ID[:7], commit.URL,
strings.TrimRight(commit.Message, "\r\n"), commit.Author.Name)

This comment has been minimized.

@ethantkoenig

ethantkoenig Oct 25, 2017

Member

commit.Author may be nil (see #2771)

MsgType: "actionCard",
ActionCard: dingtalk.ActionCard{
Text: title,
Title: text,

This comment has been minimized.

@ethantkoenig

ethantkoenig Oct 25, 2017

Member

Should title and text be reversed?

case DING_TALK:
payloader, err = GetDingtalkPayload(p, event, w.Meta)
if err != nil {
return fmt.Errorf("GetDiscordPayload: %v", err)

This comment has been minimized.

@ethantkoenig

ethantkoenig Oct 25, 2017

Member

Wrong error message

type (
// DingtalkPayload represents

This comment has been minimized.

@ethantkoenig

ethantkoenig Oct 25, 2017

Member

nit: represents what?

MsgType: "actionCard",
ActionCard: dingtalk.ActionCard{
Text: title,
Title: text,

This comment has been minimized.

@ethantkoenig

ethantkoenig Oct 25, 2017

Member

Should title and text be reversed?

lunny added some commits Oct 25, 2017

@lunny lunny force-pushed the lunny:lunny/webhook_ding_talk branch from f33e2e3 to 979991f Oct 25, 2017

lunny added some commits Oct 25, 2017

@codecov-io

This comment has been minimized.

codecov-io commented Oct 25, 2017

Codecov Report

Merging #2777 into master will decrease coverage by 0.24%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2777      +/-   ##
==========================================
- Coverage   27.23%   26.99%   -0.25%     
==========================================
  Files          89       90       +1     
  Lines       17648    17806     +158     
==========================================
  Hits         4806     4806              
- Misses      12154    12312     +158     
  Partials      688      688
Impacted Files Coverage Δ
models/webhook_dingtalk.go 0% <0%> (ø)
models/webhook.go 46.9% <0%> (-0.91%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 420fc8e...3238af9. Read the comment docs.

@lunny

This comment has been minimized.

Member

lunny commented Oct 25, 2017

@lunny

This comment has been minimized.

Member

lunny commented Nov 20, 2017

This is ready to review for v1.4

@lafriks

This comment has been minimized.

Member

lafriks commented Nov 20, 2017

LGTM

@tboerger tboerger added lgtm/need 1 and removed lgtm/need 2 labels Nov 20, 2017

@ethantkoenig

This comment has been minimized.

Member

ethantkoenig commented Nov 21, 2017

LGTM

@tboerger tboerger added lgtm/done and removed lgtm/need 1 labels Nov 21, 2017

lunny added some commits Nov 21, 2017

@lafriks lafriks merged commit 10b54df into go-gitea:master Nov 21, 2017

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr the build was successful
Details

@lunny lunny deleted the lunny:lunny/webhook_ding_talk branch Nov 21, 2017

@lunny lunny referenced this pull request Dec 8, 2017

Closed

Will the support of Dingtalk be considered? #3118

2 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment