search

lgtmco / lgtm

LGTM is a simple pull request approval system [ARCHIVE]
Apache License 2.0
988 stars 140 forks source link

Trouble with pattern match from examples #30

Open orangejulius opened 8 years ago

orangejulius commented 8 years ago

Hello, We've been using LGTM with the Pelias project for a few weeks, and overall it's been a nice way to handle signoff on our PRs, so thank you.

There's one thing we can't figure out though: we put pattern = "(?i):shipit:|:\\+1:|LGTM" in our .lgtm file as shown in the customization docs, and it works great for registering comments with LGTM and :shipit: , but not :+1:. We can't figure out why this is, and have tried to eliminate all the possibilities.

Example PRs

Working with :shipit: or LGTM

First, an example pull request where everything worked great. Both reviewers responded with :shipit:, and as you can see in the merge details, the LGTM check passed. You can see the .lgtm file is also a direct copy of the config example from the docs. Comments with LGTM(although I can't find an example PR) or a combination of the two work fine too.

Broken with 2 :+1:'s

In another pull request, both reviewers responded with :+1:, and the check failed. It's a little hard to see in the pull request itself, but it only lists our two travis checks as passing. If you find the PR in the list of closed PRs, and hover your cursor over the yellow circle, you'll see there was a 3rd check that failed, the LGTM one. It's a different repo but the .lgtm file is exactly the same. screenshot_2016-07-10_17-56-28

Sometimes broken with :+1: and LGTM or :shipit:

This is the weirdest one. Here's an example where it works, and another where it doesn't.

The regex seems to work

The weird thing is the regex from the docs seems to work in the Go Playground, so I'm not sure where the problem is. I'm happy to paste info from webhook calls if that helps, or make any other config changes to our repos you'd like to test things out. Any ideas?

bradrydzewski commented 8 years ago

@orangejulius please see the faq which has a few different items for troubleshooting https://lgtm.co/docs/faq/ ... specifically checking the LGTM response to the GitHub hook. That should provide additional information for debugging. Note also that LGTM can be considered deterministic with the exception that makes API calls to GitHub. So I would not expect inconsistent behavior unless there were issues interacting with GitHub. If this is the case it would likely be reported in the response.

Note that I also recommend digging into the source code as well. This is the best way to debug as I generally only provide high level support for this project due to time constraints.

note: edited my reply because it wasn't super polite, sorry about that. Feeling some serious open source burnout at the moment

orangejulius commented 8 years ago

Hey, no worries, I understand all about being busy. I'm sorry my comment made it look like I just wanted free support. I care much less about that than helping get a possible bug fixed for everyone.

Anyways I dug into the webhook responses for https://github.com/pelias/whosonfirst/pull/105 since they are still available. I included a quick summary of events and then the full webhook bodies (I think only a small part of the body is relevant but this way there's no possibility of missing something).

quick summary of the flow in the PR

  1. I comment with 'LGTM'. Webhooks request shows the comment body as expected, webhook response shows the proper pattern, and that I was marked as one approver. The response also shows that the PR is not yet approved, since the number of required approvals is 2. All as expected so far.
  2. Another person comments with ':+1'. The webhook request body shows the comment as expected, but there is still only one approver listed (me), and the PR remains unapproved
  3. I comment again with :shipit:. This time we both show up as approvers and the PR is in fact approved.

The comments were spaced out by at least several minutes, and not edited, so I'm not sue what could be going on. The response code for all of them was 200 and the .lgtm file is a month old, much older than the PR, so I think all the bases are covered and that this really may be a bug. If there's something I missed I'm happy to check it out though. I will try to look into the source in the near future if I have the time but if you have any ideas of where to look it would probably help speed things up.

First comment request body

{
  "action": "created",
  "issue": {
    "url": "https://api.github.com/repos/pelias/whosonfirst/issues/105",
    "repository_url": "https://api.github.com/repos/pelias/whosonfirst",
    "labels_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105/labels{/name}",
    "comments_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105/comments",
    "events_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105/events",
    "html_url": "https://github.com/pelias/whosonfirst/pull/105",
    "id": 162993633,
    "number": 105,
    "title": "added helpful error message about git-lfs",
    "user": {
      "login": "trescube",
      "id": 658991,
      "avatar_url": "https://avatars.githubusercontent.com/u/658991?v=3",
      "gravatar_id": "",
      "url": "https://api.github.com/users/trescube",
      "html_url": "https://github.com/trescube",
      "followers_url": "https://api.github.com/users/trescube/followers",
      "following_url": "https://api.github.com/users/trescube/following{/other_user}",
      "gists_url": "https://api.github.com/users/trescube/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/trescube/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/trescube/subscriptions",
      "organizations_url": "https://api.github.com/users/trescube/orgs",
      "repos_url": "https://api.github.com/users/trescube/repos",
      "events_url": "https://api.github.com/users/trescube/events{/privacy}",
      "received_events_url": "https://api.github.com/users/trescube/received_events",
      "type": "User",
      "site_admin": false
    },
    "labels": [
      {
        "url": "https://api.github.com/repos/pelias/whosonfirst/labels/in%20review",
        "name": "in review",
        "color": "ededed"
      }
    ],
    "state": "open",
    "locked": false,
    "assignee": null,
    "assignees": [

    ],
    "milestone": null,
    "comments": 3,
    "created_at": "2016-06-29T18:23:51Z",
    "updated_at": "2016-06-30T14:01:55Z",
    "closed_at": null,
    "pull_request": {
      "url": "https://api.github.com/repos/pelias/whosonfirst/pulls/105",
      "html_url": "https://github.com/pelias/whosonfirst/pull/105",
      "diff_url": "https://github.com/pelias/whosonfirst/pull/105.diff",
      "patch_url": "https://github.com/pelias/whosonfirst/pull/105.patch"
    },
    "body": "The additional error message is targeted to users who failed to take https://github.com/whosonfirst/whosonfirst-data/blob/master/README.md (specifically https://github.com/whosonfirst/whosonfirst-data/blob/master/README.md#git-and-large-files) under advisement when cloning WOF data and encounter recoverable errors during import.\r\n\r\n#103 "
  },
  "comment": {
    "url": "https://api.github.com/repos/pelias/whosonfirst/issues/comments/229667291",
    "html_url": "https://github.com/pelias/whosonfirst/pull/105#issuecomment-229667291",
    "issue_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105",
    "id": 229667291,
    "user": {
      "login": "orangejulius",
      "id": 111716,
      "avatar_url": "https://avatars.githubusercontent.com/u/111716?v=3",
      "gravatar_id": "",
      "url": "https://api.github.com/users/orangejulius",
      "html_url": "https://github.com/orangejulius",
      "followers_url": "https://api.github.com/users/orangejulius/followers",
      "following_url": "https://api.github.com/users/orangejulius/following{/other_user}",
      "gists_url": "https://api.github.com/users/orangejulius/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/orangejulius/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/orangejulius/subscriptions",
      "organizations_url": "https://api.github.com/users/orangejulius/orgs",
      "repos_url": "https://api.github.com/users/orangejulius/repos",
      "events_url": "https://api.github.com/users/orangejulius/events{/privacy}",
      "received_events_url": "https://api.github.com/users/orangejulius/received_events",
      "type": "User",
      "site_admin": false
    },
    "created_at": "2016-06-30T14:01:55Z",
    "updated_at": "2016-06-30T14:01:55Z",
    "body": ":shipit: "
  },
  "repository": {
    "id": 43167935,
    "name": "whosonfirst",
    "full_name": "pelias/whosonfirst",
    "owner": {
      "login": "pelias",
      "id": 8240367,
      "avatar_url": "https://avatars.githubusercontent.com/u/8240367?v=3",
      "gravatar_id": "",
      "url": "https://api.github.com/users/pelias",
      "html_url": "https://github.com/pelias",
      "followers_url": "https://api.github.com/users/pelias/followers",
      "following_url": "https://api.github.com/users/pelias/following{/other_user}",
      "gists_url": "https://api.github.com/users/pelias/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/pelias/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/pelias/subscriptions",
      "organizations_url": "https://api.github.com/users/pelias/orgs",
      "repos_url": "https://api.github.com/users/pelias/repos",
      "events_url": "https://api.github.com/users/pelias/events{/privacy}",
      "received_events_url": "https://api.github.com/users/pelias/received_events",
      "type": "Organization",
      "site_admin": false
    },
    "private": false,
    "html_url": "https://github.com/pelias/whosonfirst",
    "description": "Importer for Who's on First gazetteer",
    "fork": false,
    "url": "https://api.github.com/repos/pelias/whosonfirst",
    "forks_url": "https://api.github.com/repos/pelias/whosonfirst/forks",
    "keys_url": "https://api.github.com/repos/pelias/whosonfirst/keys{/key_id}",
    "collaborators_url": "https://api.github.com/repos/pelias/whosonfirst/collaborators{/collaborator}",
    "teams_url": "https://api.github.com/repos/pelias/whosonfirst/teams",
    "hooks_url": "https://api.github.com/repos/pelias/whosonfirst/hooks",
    "issue_events_url": "https://api.github.com/repos/pelias/whosonfirst/issues/events{/number}",
    "events_url": "https://api.github.com/repos/pelias/whosonfirst/events",
    "assignees_url": "https://api.github.com/repos/pelias/whosonfirst/assignees{/user}",
    "branches_url": "https://api.github.com/repos/pelias/whosonfirst/branches{/branch}",
    "tags_url": "https://api.github.com/repos/pelias/whosonfirst/tags",
    "blobs_url": "https://api.github.com/repos/pelias/whosonfirst/git/blobs{/sha}",
    "git_tags_url": "https://api.github.com/repos/pelias/whosonfirst/git/tags{/sha}",
    "git_refs_url": "https://api.github.com/repos/pelias/whosonfirst/git/refs{/sha}",
    "trees_url": "https://api.github.com/repos/pelias/whosonfirst/git/trees{/sha}",
    "statuses_url": "https://api.github.com/repos/pelias/whosonfirst/statuses/{sha}",
    "languages_url": "https://api.github.com/repos/pelias/whosonfirst/languages",
    "stargazers_url": "https://api.github.com/repos/pelias/whosonfirst/stargazers",
    "contributors_url": "https://api.github.com/repos/pelias/whosonfirst/contributors",
    "subscribers_url": "https://api.github.com/repos/pelias/whosonfirst/subscribers",
    "subscription_url": "https://api.github.com/repos/pelias/whosonfirst/subscription",
    "commits_url": "https://api.github.com/repos/pelias/whosonfirst/commits{/sha}",
    "git_commits_url": "https://api.github.com/repos/pelias/whosonfirst/git/commits{/sha}",
    "comments_url": "https://api.github.com/repos/pelias/whosonfirst/comments{/number}",
    "issue_comment_url": "https://api.github.com/repos/pelias/whosonfirst/issues/comments{/number}",
    "contents_url": "https://api.github.com/repos/pelias/whosonfirst/contents/{+path}",
    "compare_url": "https://api.github.com/repos/pelias/whosonfirst/compare/{base}...{head}",
    "merges_url": "https://api.github.com/repos/pelias/whosonfirst/merges",
    "archive_url": "https://api.github.com/repos/pelias/whosonfirst/{archive_format}{/ref}",
    "downloads_url": "https://api.github.com/repos/pelias/whosonfirst/downloads",
    "issues_url": "https://api.github.com/repos/pelias/whosonfirst/issues{/number}",
    "pulls_url": "https://api.github.com/repos/pelias/whosonfirst/pulls{/number}",
    "milestones_url": "https://api.github.com/repos/pelias/whosonfirst/milestones{/number}",
    "notifications_url": "https://api.github.com/repos/pelias/whosonfirst/notifications{?since,all,participating}",
    "labels_url": "https://api.github.com/repos/pelias/whosonfirst/labels{/name}",
    "releases_url": "https://api.github.com/repos/pelias/whosonfirst/releases{/id}",
    "deployments_url": "https://api.github.com/repos/pelias/whosonfirst/deployments",
    "created_at": "2015-09-25T18:21:06Z",
    "updated_at": "2016-01-12T19:17:35Z",
    "pushed_at": "2016-06-29T18:23:51Z",
    "git_url": "git://github.com/pelias/whosonfirst.git",
    "ssh_url": "git@github.com:pelias/whosonfirst.git",
    "clone_url": "https://github.com/pelias/whosonfirst.git",
    "svn_url": "https://github.com/pelias/whosonfirst",
    "homepage": "",
    "size": 147,
    "stargazers_count": 0,
    "watchers_count": 0,
    "language": "JavaScript",
    "has_issues": true,
    "has_downloads": true,
    "has_wiki": true,
    "has_pages": false,
    "forks_count": 1,
    "mirror_url": null,
    "open_issues_count": 12,
    "forks": 1,
    "open_issues": 12,
    "watchers": 0,
    "default_branch": "master"
  },
  "organization": {
    "login": "pelias",
    "id": 8240367,
    "url": "https://api.github.com/orgs/pelias",
    "repos_url": "https://api.github.com/orgs/pelias/repos",
    "events_url": "https://api.github.com/orgs/pelias/events",
    "hooks_url": "https://api.github.com/orgs/pelias/hooks",
    "issues_url": "https://api.github.com/orgs/pelias/issues",
    "members_url": "https://api.github.com/orgs/pelias/members{/member}",
    "public_members_url": "https://api.github.com/orgs/pelias/public_members{/member}",
    "avatar_url": "https://avatars.githubusercontent.com/u/8240367?v=3",
    "description": "A distributed full-text geographic search engine. An open source project sponsored by Mapzen."
  },
  "sender": {
    "login": "orangejulius",
    "id": 111716,
    "avatar_url": "https://avatars.githubusercontent.com/u/111716?v=3",
    "gravatar_id": "",
    "url": "https://api.github.com/users/orangejulius",
    "html_url": "https://github.com/orangejulius",
    "followers_url": "https://api.github.com/users/orangejulius/followers",
    "following_url": "https://api.github.com/users/orangejulius/following{/other_user}",
    "gists_url": "https://api.github.com/users/orangejulius/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/orangejulius/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/orangejulius/subscriptions",
    "organizations_url": "https://api.github.com/users/orangejulius/orgs",
    "repos_url": "https://api.github.com/users/orangejulius/repos",
    "events_url": "https://api.github.com/users/orangejulius/events{/privacy}",
    "received_events_url": "https://api.github.com/users/orangejulius/received_events",
    "type": "User",
    "site_admin": false
  }
}

first comment webhook response

{
    "approved": false,
    "approved_by": [
        {
            "name": "",
            "email": "",
            "login": "orangejulius"
        }
    ],
    "approvers": {
        "avulfson17": {
            "name": "",
            "email": "",
            "login": "avulfson17"
        },
        "dianashk": {
            "name": "",
            "email": "",
            "login": "dianashk"
        },
        "missinglink": {
            "name": "",
            "email": "",
            "login": "missinglink"
        },
        "orangejulius": {
            "name": "",
            "email": "",
            "login": "orangejulius"
        },
        "trescube": {
            "name": "",
            "email": "",
            "login": "trescube"
        }
    },
    "settings": {
        "approvals": 2,
        "pattern": "(?i):shipit:|:\\+1:|LGTM"
    }
}

Second comment webhook request

{
  "action": "created",
  "issue": {
    "url": "https://api.github.com/repos/pelias/whosonfirst/issues/105",
    "repository_url": "https://api.github.com/repos/pelias/whosonfirst",
    "labels_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105/labels{/name}",
    "comments_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105/comments",
    "events_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105/events",
    "html_url": "https://github.com/pelias/whosonfirst/pull/105",
    "id": 162993633,
    "number": 105,
    "title": "added helpful error message about git-lfs",
    "user": {
      "login": "trescube",
      "id": 658991,
      "avatar_url": "https://avatars.githubusercontent.com/u/658991?v=3",
      "gravatar_id": "",
      "url": "https://api.github.com/users/trescube",
      "html_url": "https://github.com/trescube",
      "followers_url": "https://api.github.com/users/trescube/followers",
      "following_url": "https://api.github.com/users/trescube/following{/other_user}",
      "gists_url": "https://api.github.com/users/trescube/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/trescube/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/trescube/subscriptions",
      "organizations_url": "https://api.github.com/users/trescube/orgs",
      "repos_url": "https://api.github.com/users/trescube/repos",
      "events_url": "https://api.github.com/users/trescube/events{/privacy}",
      "received_events_url": "https://api.github.com/users/trescube/received_events",
      "type": "User",
      "site_admin": false
    },
    "labels": [
      {
        "url": "https://api.github.com/repos/pelias/whosonfirst/labels/in%20review",
        "name": "in review",
        "color": "ededed"
      }
    ],
    "state": "open",
    "locked": false,
    "assignee": null,
    "assignees": [

    ],
    "milestone": null,
    "comments": 2,
    "created_at": "2016-06-29T18:23:51Z",
    "updated_at": "2016-06-30T13:42:28Z",
    "closed_at": null,
    "pull_request": {
      "url": "https://api.github.com/repos/pelias/whosonfirst/pulls/105",
      "html_url": "https://github.com/pelias/whosonfirst/pull/105",
      "diff_url": "https://github.com/pelias/whosonfirst/pull/105.diff",
      "patch_url": "https://github.com/pelias/whosonfirst/pull/105.patch"
    },
    "body": "The additional error message is targeted to users who failed to take https://github.com/whosonfirst/whosonfirst-data/blob/master/README.md (specifically https://github.com/whosonfirst/whosonfirst-data/blob/master/README.md#git-and-large-files) under advisement when cloning WOF data and encounter recoverable errors during import.\r\n\r\n#103 "
  },
  "comment": {
    "url": "https://api.github.com/repos/pelias/whosonfirst/issues/comments/229661591",
    "html_url": "https://github.com/pelias/whosonfirst/pull/105#issuecomment-229661591",
    "issue_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105",
    "id": 229661591,
    "user": {
      "login": "missinglink",
      "id": 738069,
      "avatar_url": "https://avatars.githubusercontent.com/u/738069?v=3",
      "gravatar_id": "",
      "url": "https://api.github.com/users/missinglink",
      "html_url": "https://github.com/missinglink",
      "followers_url": "https://api.github.com/users/missinglink/followers",
      "following_url": "https://api.github.com/users/missinglink/following{/other_user}",
      "gists_url": "https://api.github.com/users/missinglink/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/missinglink/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/missinglink/subscriptions",
      "organizations_url": "https://api.github.com/users/missinglink/orgs",
      "repos_url": "https://api.github.com/users/missinglink/repos",
      "events_url": "https://api.github.com/users/missinglink/events{/privacy}",
      "received_events_url": "https://api.github.com/users/missinglink/received_events",
      "type": "User",
      "site_admin": false
    },
    "created_at": "2016-06-30T13:42:28Z",
    "updated_at": "2016-06-30T13:42:28Z",
    "body": ":+1:"
  },
  "repository": {
    "id": 43167935,
    "name": "whosonfirst",
    "full_name": "pelias/whosonfirst",
    "owner": {
      "login": "pelias",
      "id": 8240367,
      "avatar_url": "https://avatars.githubusercontent.com/u/8240367?v=3",
      "gravatar_id": "",
      "url": "https://api.github.com/users/pelias",
      "html_url": "https://github.com/pelias",
      "followers_url": "https://api.github.com/users/pelias/followers",
      "following_url": "https://api.github.com/users/pelias/following{/other_user}",
      "gists_url": "https://api.github.com/users/pelias/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/pelias/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/pelias/subscriptions",
      "organizations_url": "https://api.github.com/users/pelias/orgs",
      "repos_url": "https://api.github.com/users/pelias/repos",
      "events_url": "https://api.github.com/users/pelias/events{/privacy}",
      "received_events_url": "https://api.github.com/users/pelias/received_events",
      "type": "Organization",
      "site_admin": false
    },
    "private": false,
    "html_url": "https://github.com/pelias/whosonfirst",
    "description": "Importer for Who's on First gazetteer",
    "fork": false,
    "url": "https://api.github.com/repos/pelias/whosonfirst",
    "forks_url": "https://api.github.com/repos/pelias/whosonfirst/forks",
    "keys_url": "https://api.github.com/repos/pelias/whosonfirst/keys{/key_id}",
    "collaborators_url": "https://api.github.com/repos/pelias/whosonfirst/collaborators{/collaborator}",
    "teams_url": "https://api.github.com/repos/pelias/whosonfirst/teams",
    "hooks_url": "https://api.github.com/repos/pelias/whosonfirst/hooks",
    "issue_events_url": "https://api.github.com/repos/pelias/whosonfirst/issues/events{/number}",
    "events_url": "https://api.github.com/repos/pelias/whosonfirst/events",
    "assignees_url": "https://api.github.com/repos/pelias/whosonfirst/assignees{/user}",
    "branches_url": "https://api.github.com/repos/pelias/whosonfirst/branches{/branch}",
    "tags_url": "https://api.github.com/repos/pelias/whosonfirst/tags",
    "blobs_url": "https://api.github.com/repos/pelias/whosonfirst/git/blobs{/sha}",
    "git_tags_url": "https://api.github.com/repos/pelias/whosonfirst/git/tags{/sha}",
    "git_refs_url": "https://api.github.com/repos/pelias/whosonfirst/git/refs{/sha}",
    "trees_url": "https://api.github.com/repos/pelias/whosonfirst/git/trees{/sha}",
    "statuses_url": "https://api.github.com/repos/pelias/whosonfirst/statuses/{sha}",
    "languages_url": "https://api.github.com/repos/pelias/whosonfirst/languages",
    "stargazers_url": "https://api.github.com/repos/pelias/whosonfirst/stargazers",
    "contributors_url": "https://api.github.com/repos/pelias/whosonfirst/contributors",
    "subscribers_url": "https://api.github.com/repos/pelias/whosonfirst/subscribers",
    "subscription_url": "https://api.github.com/repos/pelias/whosonfirst/subscription",
    "commits_url": "https://api.github.com/repos/pelias/whosonfirst/commits{/sha}",
    "git_commits_url": "https://api.github.com/repos/pelias/whosonfirst/git/commits{/sha}",
    "comments_url": "https://api.github.com/repos/pelias/whosonfirst/comments{/number}",
    "issue_comment_url": "https://api.github.com/repos/pelias/whosonfirst/issues/comments{/number}",
    "contents_url": "https://api.github.com/repos/pelias/whosonfirst/contents/{+path}",
    "compare_url": "https://api.github.com/repos/pelias/whosonfirst/compare/{base}...{head}",
    "merges_url": "https://api.github.com/repos/pelias/whosonfirst/merges",
    "archive_url": "https://api.github.com/repos/pelias/whosonfirst/{archive_format}{/ref}",
    "downloads_url": "https://api.github.com/repos/pelias/whosonfirst/downloads",
    "issues_url": "https://api.github.com/repos/pelias/whosonfirst/issues{/number}",
    "pulls_url": "https://api.github.com/repos/pelias/whosonfirst/pulls{/number}",
    "milestones_url": "https://api.github.com/repos/pelias/whosonfirst/milestones{/number}",
    "notifications_url": "https://api.github.com/repos/pelias/whosonfirst/notifications{?since,all,participating}",
    "labels_url": "https://api.github.com/repos/pelias/whosonfirst/labels{/name}",
    "releases_url": "https://api.github.com/repos/pelias/whosonfirst/releases{/id}",
    "deployments_url": "https://api.github.com/repos/pelias/whosonfirst/deployments",
    "created_at": "2015-09-25T18:21:06Z",
    "updated_at": "2016-01-12T19:17:35Z",
    "pushed_at": "2016-06-29T18:23:51Z",
    "git_url": "git://github.com/pelias/whosonfirst.git",
    "ssh_url": "git@github.com:pelias/whosonfirst.git",
    "clone_url": "https://github.com/pelias/whosonfirst.git",
    "svn_url": "https://github.com/pelias/whosonfirst",
    "homepage": "",
    "size": 147,
    "stargazers_count": 0,
    "watchers_count": 0,
    "language": "JavaScript",
    "has_issues": true,
    "has_downloads": true,
    "has_wiki": true,
    "has_pages": false,
    "forks_count": 1,
    "mirror_url": null,
    "open_issues_count": 12,
    "forks": 1,
    "open_issues": 12,
    "watchers": 0,
    "default_branch": "master"
  },
  "organization": {
    "login": "pelias",
    "id": 8240367,
    "url": "https://api.github.com/orgs/pelias",
    "repos_url": "https://api.github.com/orgs/pelias/repos",
    "events_url": "https://api.github.com/orgs/pelias/events",
    "hooks_url": "https://api.github.com/orgs/pelias/hooks",
    "issues_url": "https://api.github.com/orgs/pelias/issues",
    "members_url": "https://api.github.com/orgs/pelias/members{/member}",
    "public_members_url": "https://api.github.com/orgs/pelias/public_members{/member}",
    "avatar_url": "https://avatars.githubusercontent.com/u/8240367?v=3",
    "description": "A distributed full-text geographic search engine. An open source project sponsored by Mapzen."
  },
  "sender": {
    "login": "missinglink",
    "id": 738069,
    "avatar_url": "https://avatars.githubusercontent.com/u/738069?v=3",
    "gravatar_id": "",
    "url": "https://api.github.com/users/missinglink",
    "html_url": "https://github.com/missinglink",
    "followers_url": "https://api.github.com/users/missinglink/followers",
    "following_url": "https://api.github.com/users/missinglink/following{/other_user}",
    "gists_url": "https://api.github.com/users/missinglink/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/missinglink/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/missinglink/subscriptions",
    "organizations_url": "https://api.github.com/users/missinglink/orgs",
    "repos_url": "https://api.github.com/users/missinglink/repos",
    "events_url": "https://api.github.com/users/missinglink/events{/privacy}",
    "received_events_url": "https://api.github.com/users/missinglink/received_events",
    "type": "User",
    "site_admin": false
  }
}

Second comment webhook response

{
    "approved": false,
    "approved_by": [
        {
            "name": "",
            "email": "",
            "login": "orangejulius"
        }
    ],
    "approvers": {
        "avulfson17": {
            "name": "",
            "email": "",
            "login": "avulfson17"
        },
        "dianashk": {
            "name": "",
            "email": "",
            "login": "dianashk"
        },
        "missinglink": {
            "name": "",
            "email": "",
            "login": "missinglink"
        },
        "orangejulius": {
            "name": "",
            "email": "",
            "login": "orangejulius"
        },
        "trescube": {
            "name": "",
            "email": "",
            "login": "trescube"
        }
    },
    "settings": {
        "approvals": 2,
        "pattern": "(?i):shipit:|:\\+1:|LGTM"
    }
}

Third comment webhook request

{
  "action": "created",
  "issue": {
    "url": "https://api.github.com/repos/pelias/whosonfirst/issues/105",
    "repository_url": "https://api.github.com/repos/pelias/whosonfirst",
    "labels_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105/labels{/name}",
    "comments_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105/comments",
    "events_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105/events",
    "html_url": "https://github.com/pelias/whosonfirst/pull/105",
    "id": 162993633,
    "number": 105,
    "title": "added helpful error message about git-lfs",
    "user": {
      "login": "trescube",
      "id": 658991,
      "avatar_url": "https://avatars.githubusercontent.com/u/658991?v=3",
      "gravatar_id": "",
      "url": "https://api.github.com/users/trescube",
      "html_url": "https://github.com/trescube",
      "followers_url": "https://api.github.com/users/trescube/followers",
      "following_url": "https://api.github.com/users/trescube/following{/other_user}",
      "gists_url": "https://api.github.com/users/trescube/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/trescube/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/trescube/subscriptions",
      "organizations_url": "https://api.github.com/users/trescube/orgs",
      "repos_url": "https://api.github.com/users/trescube/repos",
      "events_url": "https://api.github.com/users/trescube/events{/privacy}",
      "received_events_url": "https://api.github.com/users/trescube/received_events",
      "type": "User",
      "site_admin": false
    },
    "labels": [
      {
        "url": "https://api.github.com/repos/pelias/whosonfirst/labels/in%20review",
        "name": "in review",
        "color": "ededed"
      }
    ],
    "state": "open",
    "locked": false,
    "assignee": null,
    "assignees": [

    ],
    "milestone": null,
    "comments": 3,
    "created_at": "2016-06-29T18:23:51Z",
    "updated_at": "2016-06-30T14:01:55Z",
    "closed_at": null,
    "pull_request": {
      "url": "https://api.github.com/repos/pelias/whosonfirst/pulls/105",
      "html_url": "https://github.com/pelias/whosonfirst/pull/105",
      "diff_url": "https://github.com/pelias/whosonfirst/pull/105.diff",
      "patch_url": "https://github.com/pelias/whosonfirst/pull/105.patch"
    },
    "body": "The additional error message is targeted to users who failed to take https://github.com/whosonfirst/whosonfirst-data/blob/master/README.md (specifically https://github.com/whosonfirst/whosonfirst-data/blob/master/README.md#git-and-large-files) under advisement when cloning WOF data and encounter recoverable errors during import.\r\n\r\n#103 "
  },
  "comment": {
    "url": "https://api.github.com/repos/pelias/whosonfirst/issues/comments/229667291",
    "html_url": "https://github.com/pelias/whosonfirst/pull/105#issuecomment-229667291",
    "issue_url": "https://api.github.com/repos/pelias/whosonfirst/issues/105",
    "id": 229667291,
    "user": {
      "login": "orangejulius",
      "id": 111716,
      "avatar_url": "https://avatars.githubusercontent.com/u/111716?v=3",
      "gravatar_id": "",
      "url": "https://api.github.com/users/orangejulius",
      "html_url": "https://github.com/orangejulius",
      "followers_url": "https://api.github.com/users/orangejulius/followers",
      "following_url": "https://api.github.com/users/orangejulius/following{/other_user}",
      "gists_url": "https://api.github.com/users/orangejulius/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/orangejulius/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/orangejulius/subscriptions",
      "organizations_url": "https://api.github.com/users/orangejulius/orgs",
      "repos_url": "https://api.github.com/users/orangejulius/repos",
      "events_url": "https://api.github.com/users/orangejulius/events{/privacy}",
      "received_events_url": "https://api.github.com/users/orangejulius/received_events",
      "type": "User",
      "site_admin": false
    },
    "created_at": "2016-06-30T14:01:55Z",
    "updated_at": "2016-06-30T14:01:55Z",
    "body": ":shipit: "
  },
  "repository": {
    "id": 43167935,
    "name": "whosonfirst",
    "full_name": "pelias/whosonfirst",
    "owner": {
      "login": "pelias",
      "id": 8240367,
      "avatar_url": "https://avatars.githubusercontent.com/u/8240367?v=3",
      "gravatar_id": "",
      "url": "https://api.github.com/users/pelias",
      "html_url": "https://github.com/pelias",
      "followers_url": "https://api.github.com/users/pelias/followers",
      "following_url": "https://api.github.com/users/pelias/following{/other_user}",
      "gists_url": "https://api.github.com/users/pelias/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/pelias/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/pelias/subscriptions",
      "organizations_url": "https://api.github.com/users/pelias/orgs",
      "repos_url": "https://api.github.com/users/pelias/repos",
      "events_url": "https://api.github.com/users/pelias/events{/privacy}",
      "received_events_url": "https://api.github.com/users/pelias/received_events",
      "type": "Organization",
      "site_admin": false
    },
    "private": false,
    "html_url": "https://github.com/pelias/whosonfirst",
    "description": "Importer for Who's on First gazetteer",
    "fork": false,
    "url": "https://api.github.com/repos/pelias/whosonfirst",
    "forks_url": "https://api.github.com/repos/pelias/whosonfirst/forks",
    "keys_url": "https://api.github.com/repos/pelias/whosonfirst/keys{/key_id}",
    "collaborators_url": "https://api.github.com/repos/pelias/whosonfirst/collaborators{/collaborator}",
    "teams_url": "https://api.github.com/repos/pelias/whosonfirst/teams",
    "hooks_url": "https://api.github.com/repos/pelias/whosonfirst/hooks",
    "issue_events_url": "https://api.github.com/repos/pelias/whosonfirst/issues/events{/number}",
    "events_url": "https://api.github.com/repos/pelias/whosonfirst/events",
    "assignees_url": "https://api.github.com/repos/pelias/whosonfirst/assignees{/user}",
    "branches_url": "https://api.github.com/repos/pelias/whosonfirst/branches{/branch}",
    "tags_url": "https://api.github.com/repos/pelias/whosonfirst/tags",
    "blobs_url": "https://api.github.com/repos/pelias/whosonfirst/git/blobs{/sha}",
    "git_tags_url": "https://api.github.com/repos/pelias/whosonfirst/git/tags{/sha}",
    "git_refs_url": "https://api.github.com/repos/pelias/whosonfirst/git/refs{/sha}",
    "trees_url": "https://api.github.com/repos/pelias/whosonfirst/git/trees{/sha}",
    "statuses_url": "https://api.github.com/repos/pelias/whosonfirst/statuses/{sha}",
    "languages_url": "https://api.github.com/repos/pelias/whosonfirst/languages",
    "stargazers_url": "https://api.github.com/repos/pelias/whosonfirst/stargazers",
    "contributors_url": "https://api.github.com/repos/pelias/whosonfirst/contributors",
    "subscribers_url": "https://api.github.com/repos/pelias/whosonfirst/subscribers",
    "subscription_url": "https://api.github.com/repos/pelias/whosonfirst/subscription",
    "commits_url": "https://api.github.com/repos/pelias/whosonfirst/commits{/sha}",
    "git_commits_url": "https://api.github.com/repos/pelias/whosonfirst/git/commits{/sha}",
    "comments_url": "https://api.github.com/repos/pelias/whosonfirst/comments{/number}",
    "issue_comment_url": "https://api.github.com/repos/pelias/whosonfirst/issues/comments{/number}",
    "contents_url": "https://api.github.com/repos/pelias/whosonfirst/contents/{+path}",
    "compare_url": "https://api.github.com/repos/pelias/whosonfirst/compare/{base}...{head}",
    "merges_url": "https://api.github.com/repos/pelias/whosonfirst/merges",
    "archive_url": "https://api.github.com/repos/pelias/whosonfirst/{archive_format}{/ref}",
    "downloads_url": "https://api.github.com/repos/pelias/whosonfirst/downloads",
    "issues_url": "https://api.github.com/repos/pelias/whosonfirst/issues{/number}",
    "pulls_url": "https://api.github.com/repos/pelias/whosonfirst/pulls{/number}",
    "milestones_url": "https://api.github.com/repos/pelias/whosonfirst/milestones{/number}",
    "notifications_url": "https://api.github.com/repos/pelias/whosonfirst/notifications{?since,all,participating}",
    "labels_url": "https://api.github.com/repos/pelias/whosonfirst/labels{/name}",
    "releases_url": "https://api.github.com/repos/pelias/whosonfirst/releases{/id}",
    "deployments_url": "https://api.github.com/repos/pelias/whosonfirst/deployments",
    "created_at": "2015-09-25T18:21:06Z",
    "updated_at": "2016-01-12T19:17:35Z",
    "pushed_at": "2016-06-29T18:23:51Z",
    "git_url": "git://github.com/pelias/whosonfirst.git",
    "ssh_url": "git@github.com:pelias/whosonfirst.git",
    "clone_url": "https://github.com/pelias/whosonfirst.git",
    "svn_url": "https://github.com/pelias/whosonfirst",
    "homepage": "",
    "size": 147,
    "stargazers_count": 0,
    "watchers_count": 0,
    "language": "JavaScript",
    "has_issues": true,
    "has_downloads": true,
    "has_wiki": true,
    "has_pages": false,
    "forks_count": 1,
    "mirror_url": null,
    "open_issues_count": 12,
    "forks": 1,
    "open_issues": 12,
    "watchers": 0,
    "default_branch": "master"
  },
  "organization": {
    "login": "pelias",
    "id": 8240367,
    "url": "https://api.github.com/orgs/pelias",
    "repos_url": "https://api.github.com/orgs/pelias/repos",
    "events_url": "https://api.github.com/orgs/pelias/events",
    "hooks_url": "https://api.github.com/orgs/pelias/hooks",
    "issues_url": "https://api.github.com/orgs/pelias/issues",
    "members_url": "https://api.github.com/orgs/pelias/members{/member}",
    "public_members_url": "https://api.github.com/orgs/pelias/public_members{/member}",
    "avatar_url": "https://avatars.githubusercontent.com/u/8240367?v=3",
    "description": "A distributed full-text geographic search engine. An open source project sponsored by Mapzen."
  },
  "sender": {
    "login": "orangejulius",
    "id": 111716,
    "avatar_url": "https://avatars.githubusercontent.com/u/111716?v=3",
    "gravatar_id": "",
    "url": "https://api.github.com/users/orangejulius",
    "html_url": "https://github.com/orangejulius",
    "followers_url": "https://api.github.com/users/orangejulius/followers",
    "following_url": "https://api.github.com/users/orangejulius/following{/other_user}",
    "gists_url": "https://api.github.com/users/orangejulius/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/orangejulius/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/orangejulius/subscriptions",
    "organizations_url": "https://api.github.com/users/orangejulius/orgs",
    "repos_url": "https://api.github.com/users/orangejulius/repos",
    "events_url": "https://api.github.com/users/orangejulius/events{/privacy}",
    "received_events_url": "https://api.github.com/users/orangejulius/received_events",
    "type": "User",
    "site_admin": false
  }
}

Third comment webhook response

{
    "approved": true,
    "approved_by": [
        {
            "name": "",
            "email": "",
            "login": "orangejulius"
        },
        {
            "name": "",
            "email": "",
            "login": "missinglink"
        }
    ],
    "approvers": {
        "avulfson17": {
            "name": "",
            "email": "",
            "login": "avulfson17"
        },
        "dianashk": {
            "name": "",
            "email": "",
            "login": "dianashk"
        },
        "missinglink": {
            "name": "",
            "email": "",
            "login": "missinglink"
        },
        "orangejulius": {
            "name": "",
            "email": "",
            "login": "orangejulius"
        },
        "trescube": {
            "name": "",
            "email": "",
            "login": "trescube"
        }
    },
    "settings": {
        "approvals": 2,
        "pattern": "(?i):shipit:|:\\+1:|LGTM"
    }
}
bradrydzewski commented 8 years ago

Hey, no worries, I understand all about being busy. I'm sorry my comment made it look like I just wanted free support. I care much less about that than helping get a possible bug fixed for everyone.

sorry about that. I had edited the above comment with an apology. I've been feeling major open source burnout this weekend. No excuse though for me not being polite.

Another person comments with ':+1'. The webhook request body shows the comment as expected, but there is still only one approver listed (me), and the PR remains unapproved

if this happens again can you do me a favor and re-submit the hook from the GitHub user interface? LGTM pulls the full list of comments from GitHub and ignores the payload it receives. Maybe GitHub serves some API calls from an eventually consistent cache? I think it would also make sense to return the list of comments with a match true / false to simplify this sort of debugging, something like:

{
    "approved": true,
    "approved_by": [ ...  ],
    "approvers": { ... },
    "comments" : [
      {
        id: 229444615,
        login: "orangejulius",
        body: "LGTM",
        match: true
      },
      {
        id: 229444615,
        login: "missinglink",
        body: "LGTM",
        match: true
      },
      {
        id: 229661592,
        login: "missinglink",
        body: "some text here",
        match: false
      }
    ],
    "settings": {
        "approvals": 2,
        "pattern": "(?i):shipit:|:\\+1:|LGTM"
    }
}
orangejulius commented 7 years ago

Hey @bradrydzewski, Thanks for the apology, that's much appreciated. But also don't worry about it, open source is hard.

I have a new PR that can serve as an example of things breaking: https://github.com/pelias/pelias-doc/pull/132

I tried resubmitting the webhook a couple times several minutes after the last comment went in, but there was no effect. How else can I help you debug this?

bradrydzewski commented 7 years ago

@orangejulius sorry for delayed reply. I'm planning on jumping back in the LGTM code next week since GitHub is introducing breaking changes to their API and LGTM needs updated. While I'm in the code I'm planning to alter the payload response and echo back the list of comments including whether or not the comment matched (true / false). Hopefully this will help shed some light.

Hang in there and I'll hopefully have an update for you mid-week. Thanks!