{"action":"created","issue":{"url":"https://api.github.com/repos/thoughtbot/hound/issues/997","labels_url":"https://api.github.com/repos/thoughtbot/hound/issues/997/labels{/name}","comments_url":"https://api.github.com/repos/thoughtbot/hound/issues/997/comments","events_url":"https://api.github.com/repos/thoughtbot/hound/issues/997/events","html_url":"https://github.com/thoughtbot/hound/issues/997","id":119528932,"number":997,"title":"Re-comment if the line has changed, but the issue wasn't resolved","user":{"login":"dkniffin","id":1680080,"avatar_url":"https://avatars.githubusercontent.com/u/1680080?v=3","gravatar_id":"","url":"https://api.github.com/users/dkniffin","html_url":"https://github.com/dkniffin","followers_url":"https://api.github.com/users/dkniffin/followers","following_url":"https://api.github.com/users/dkniffin/following{/other_user}","gists_url":"https://api.github.com/users/dkniffin/gists{/gist_id}","starred_url":"https://api.github.com/users/dkniffin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dkniffin/subscriptions","organizations_url":"https://api.github.com/users/dkniffin/orgs","repos_url":"https://api.github.com/users/dkniffin/repos","events_url":"https://api.github.com/users/dkniffin/events{/privacy}","received_events_url":"https://api.github.com/users/dkniffin/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-11-30T16:34:22Z","updated_at":"2015-12-02T12:52:00Z","closed_at":null,"body":"### Flow to reproduce the issue\r\n\r\n- Create a PR with a hound issue\r\n- (Hound will comment on the line)\r\n- Modify the line, but don't fix the issue\r\n- Push\r\n- Hound still fails, with the same comment, but you can't see it because it's in an out-dated diff.\r\n\r\n### Expected behavior\r\n\r\nIf the line changed, and it still has an issue, hound should re-comment.\r\n\r\n### Possible fix\r\n\r\nI did some sleuthing, and I believe the issue is [this method](https://github.com/thoughtbot/hound/blob/master/app/models/file_review.rb#L27).\r\n\r\n````ruby\r\ndef find_or_build_violation(line)\r\n  violations.detect { |violation| violation.line_number == line.number } ||\r\n    violations.build(line_number: line.number)\r\nend\r\n````\r\n\r\nI'm not 100% how to fix it, but I think it should be something like:\r\n\r\n````ruby\r\ndef find_or_build_violation(line)\r\n  violations.detect { |violation| violation.line_number == line.number && ! violation.outdated? } || \r\n    violations.build(line_number: line.number)\r\nend\r\n````\r\n\r\nThe hard part though, is figuring out if the violation is outdated. Perhaps the github api has something for checking this? I might investigate this more and see if I can submit a PR."},"comment":{"url":"https://api.github.com/repos/thoughtbot/hound/issues/comments/161282514","html_url":"https://github.com/thoughtbot/hound/issues/997#issuecomment-161282514","issue_url":"https://api.github.com/repos/thoughtbot/hound/issues/997","id":161282514,"user":{"login":"dkniffin","id":1680080,"avatar_url":"https://avatars.githubusercontent.com/u/1680080?v=3","gravatar_id":"","url":"https://api.github.com/users/dkniffin","html_url":"https://github.com/dkniffin","followers_url":"https://api.github.com/users/dkniffin/followers","following_url":"https://api.github.com/users/dkniffin/following{/other_user}","gists_url":"https://api.github.com/users/dkniffin/gists{/gist_id}","starred_url":"https://api.github.com/users/dkniffin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dkniffin/subscriptions","organizations_url":"https://api.github.com/users/dkniffin/orgs","repos_url":"https://api.github.com/users/dkniffin/repos","events_url":"https://api.github.com/users/dkniffin/events{/privacy}","received_events_url":"https://api.github.com/users/dkniffin/received_events","type":"User","site_admin":false},"created_at":"2015-12-02T12:52:00Z","updated_at":"2015-12-02T12:52:00Z","body":"I think that was everything we discussed.\r\n\r\nI also thought a bit more on this last night. What if we did something like:\r\n\r\n````ruby\r\n  def find_or_build_violation(line)\r\n    violation = find_violation(line)\r\n    if violation.nil? || line.changed_since?(violation.sha)\r\n      violations.build(line_number: line.number)\r\n    end\r\n  end\r\n\r\n  def find_violation(line)\r\n    violations.detect { |violation| violation.line_number == line.number }\r\n  end\r\n````\r\n\r\nThe two things here that we don't have are:\r\n1) `violation.sha` - I think this would just mean adding a column to the violations table, and saving the sha when the violation is created, right? \r\n2) `line.changed_since?(sha)` - I'm still trying to think through this part, but maybe we could determine this from the set of patches in the PR? Right now we don't have easy access to those, but with a little refactoring I think we could.\r\n\r\nThoughts?"}}