maintner: discard lock_reason in GitHub issue events
"lock_reason" is a field that was added to GitHub API v3 after maintner
was created. It's present in GitHub issue "locked" events and indicates
the reason the issue was locked, such as "off-topic", "too heated",
"resolved", etc.
Since maintner didn't have explicit handling for it, it got the default
treatment of being saved in OtherJSON just in case.
It's a non-goal for maintner to track all available data, only what is
deemed to be worth including. This field hasn't been used all this time
and seems fine to exclude until something changes.
Also fix a bug where OtherJSON wasn't even set in GitHubIssueEvent,
and improve various internal comments and log messages.
For golang/go#46180.
Change-Id: I9ebb3bf4183f78e413dfb032c94cb736a8b07e02
Reviewed-on: https://go-review.googlesource.com/c/build/+/320289
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
diff --git a/maintner/github.go b/maintner/github.go
index e2bdb02..18ecf2f 100644
--- a/maintner/github.go
+++ b/maintner/github.go
@@ -607,8 +607,8 @@
Actor *GitHubUser
Label string // for type: "unlabeled", "labeled"
- Assignee *GitHubUser // for type "assigned", "unassigned"
- Assigner *GitHubUser // for type "assigned", "unassigned"
+ Assignee *GitHubUser // for type: "assigned", "unassigned"
+ Assigner *GitHubUser // for type: "assigned", "unassigned"
Milestone string // for type: "milestoned", "demilestoned"
From, To string // for type: "renamed"
CommitID, CommitURL string // for type: "closed", "referenced" ... ?
@@ -703,6 +703,7 @@
// TODO: parse it and see if we've since learned how
// to deal with it?
log.Printf("Unknown JSON in log: %s", p.OtherJson)
+ e.OtherJSON = string(p.OtherJson)
}
if p.Label != nil {
e.Label = g.c.str(p.Label.Name)
@@ -2130,8 +2131,8 @@
return nil
}
-// parseGithubEvents parses the JSON array of GitHub events in r. It
-// does this the very manual way (using map[string]interface{})
+// parseGithubEvents parses the JSON array of GitHub issue events in r.
+// It does this the very manual way (using map[string]interface{})
// instead of using nice types because https://golang.org/issue/15314
// isn't implemented yet and also because even if it were implemented,
// this code still wants to preserve any unknown fields to store in
@@ -2235,7 +2236,8 @@
e.TeamReviewer = t
}
}
- delete(em, "node_id") // not sure what it is, but don't need to store it
+ delete(em, "node_id") // GitHub API v4 Global Node ID; don't store it.
+ delete(em, "lock_reason") // Not stored.
otherJSON, _ := json.Marshal(em)
e.OtherJSON = string(otherJSON)
@@ -2243,7 +2245,7 @@
e.OtherJSON = ""
}
if e.OtherJSON != "" {
- log.Printf("warning: storing unknown field(s) in GitHub event: %s", e.OtherJSON)
+ log.Printf("warning: storing unknown field(s) in GitHub issue event: %s", e.OtherJSON)
}
evts = append(evts, e)
}
@@ -2336,7 +2338,7 @@
}
evts, err := parseGithubReviews(res.Body)
if err != nil {
- return nil, nil, fmt.Errorf("%s: parse github pr review events: %v", u, err)
+ return nil, nil, fmt.Errorf("%s: parse github pr reviews: %v", u, err)
}
is := make([]interface{}, len(evts))
for i, v := range evts {
@@ -2373,8 +2375,8 @@
return nil
}
-// parseGithubReviews parses the JSON array of GitHub review events in r. It
-// does this the very manual way (using map[string]interface{})
+// parseGithubReviews parses the JSON array of GitHub reviews in r.
+// It does this the very manual way (using map[string]interface{})
// instead of using nice types because https://golang.org/issue/15314
// isn't implemented yet and also because even if it were implemented,
// this code still wants to preserve any unknown fields to store in
@@ -2437,7 +2439,7 @@
e.Created = e.Created.UTC()
}
- delete(em, "node_id") // not sure what it is, but don't need to store it
+ delete(em, "node_id") // GitHub API v4 Global Node ID; don't store it.
delete(em, "html_url") // not needed.
delete(em, "pull_request_url") // not needed.
delete(em, "_links") // not needed. (duplicate data of above two nodes)
@@ -2448,7 +2450,7 @@
e.OtherJSON = ""
}
if e.OtherJSON != "" {
- log.Printf("warning: storing unknown field(s) in GitHub event: %s", e.OtherJSON)
+ log.Printf("warning: storing unknown field(s) in GitHub review: %s", e.OtherJSON)
}
evts = append(evts, e)
}
diff --git a/maintner/github_test.go b/maintner/github_test.go
index 8639942..a3039e3 100644
--- a/maintner/github_test.go
+++ b/maintner/github_test.go
@@ -287,7 +287,8 @@
"event": "locked",
"commit_id": null,
"commit_url": null,
- "created_at": "2017-03-13T22:39:36Z"
+ "created_at": "2017-03-13T22:39:36Z",
+ "lock_reason": "off-topic"
}`,
e: &GitHubIssueEvent{
ID: 998144646,
@@ -482,6 +483,47 @@
RenameTo: "test-2 new name",
},
},
+
+ {
+ name: "Extra Unknown JSON",
+ j: `{
+ "id": 998144526,
+ "url": "https://api.github.com/repos/bradfitz/go-issue-mirror/issues/events/998144526",
+ "actor": {
+ "login": "bradfitz",
+ "id": 2621
+ },
+ "event": "labeled",
+ "commit_id": null,
+ "commit_url": null,
+ "created_at": "2017-03-13T22:39:28Z",
+ "label": {
+ "name": "enhancement",
+ "color": "84b6eb"
+ },
+ "random_new_field": "some new thing that GitHub API may add"
+ }
+`,
+ e: &GitHubIssueEvent{
+ ID: 998144526,
+ Type: "labeled",
+ Created: t3339("2017-03-13T22:39:28Z"),
+ Actor: &GitHubUser{
+ ID: 2621,
+ Login: "bradfitz",
+ },
+ Label: "enhancement",
+ OtherJSON: `{"random_new_field":"some new thing that GitHub API may add"}`,
+ },
+ p: &maintpb.GithubIssueEvent{
+ Id: 998144526,
+ EventType: "labeled",
+ ActorId: 2621,
+ Created: p3339("2017-03-13T22:39:28Z"),
+ Label: &maintpb.GithubLabel{Name: "enhancement"},
+ OtherJson: []byte(`{"random_new_field":"some new thing that GitHub API may add"}`),
+ },
+ },
}
var eventTypes []string
@@ -817,7 +859,7 @@
},
"submitted_at": "2018-03-22T00:26:48Z",
"commit_id" : "e4d70f7e8892f024e4ed3e8b99ee6c5a9f16e126",
- "random_key": "some random value"
+ "random_new_field": "some new thing that GitHub API may add"
}`,
e: &GitHubReview{
ID: 123456,
@@ -830,7 +872,7 @@
CommitID: "e4d70f7e8892f024e4ed3e8b99ee6c5a9f16e126",
ActorAssociation: "CONTRIBUTOR",
Created: t3339("2018-03-22T00:26:48Z"),
- OtherJSON: `{"random_key":"some random value"}`,
+ OtherJSON: `{"random_new_field":"some new thing that GitHub API may add"}`,
},
p: &maintpb.GithubReview{
Id: 123456,
@@ -840,7 +882,7 @@
CommitId: "e4d70f7e8892f024e4ed3e8b99ee6c5a9f16e126",
ActorAssociation: "CONTRIBUTOR",
Created: p3339("2018-03-22T00:26:48Z"),
- OtherJson: []byte(`{"random_key":"some random value"}`),
+ OtherJson: []byte(`{"random_new_field":"some new thing that GitHub API may add"}`),
},
},
}
