Never post the same comment twice

[darkwing]

If a pull request is submitted, then another commit pushed to that branch (thus updating the PR), the entire PR is then reevaluated and commit comments duplicated.

[darkwing]

I was thinking about this, and thinking we could read all comments from our bot and delete them via the API, but if they have replies, that would mess things up.

[Faeranne]

I do believe that we can access just the most recent commit, but since PR comments occur on the merge commit that may confuse things. Any reason we shouldn't re-comment on issues in the PR?

[darkwing]

My issue is that let's say a PR goes back and forth a dozen times...do we want a dozen comments for the same change for each update/commit? It's too much noise, IMO. We should at least dig for a better solution.

[Faeranne]

We could just ignore files that haven't changed in the last commit. Those that have will loose the original; comments, so we will need to re-comment if they haven't been fixed.

[openjck]

Lines that change will lose their comments, but only those lines. So re-commenting on files that have changed won't be enough.

[Faeranne]

The latest commit should only have lines that have changed. Perhaps we use only the latest commit to determine which lines have changes, and build comments from that? Since we would still have merge commit numbers, it should only be one additional call to github to fetch the latest commit diff.

[openjck]

We'll also need to handle the case where someone rewrites the history of a branch.

I wonder if we might be over-thinking it. Would something as simple as not adding a comment if one already exists work?

[Faeranne]

What if someone else made a comment on the line? I agree that it seems like there should be a simpler answer, but i'd bet that we could release a fix with what we have now.

As for branch rewrites? I believe Github will clear all comments when that happens, so we use the latest commit, which should have all the changes again. This is one of the reasons I feel that rewriting history should be saved for the actual PR merge, or not executed at all, but it's definitely a problem we need to solve.

[Faeranne]

If #89 gets completed, we can use the db to track comments easily.

[openjck]

This can be triggered by a few things, including but not limited to rebasing a commit, adding a new commit to the same branch, and closing/reopening a pull request.

[openjck]

Quoting @groovecoder from #151:

@openjck and I thought a solution may be:

1. Create a comments table in our shiny new postgres database
2. Update the comment code to store a hash of the url, file name, line number, and line contents
3. Before making a comment, make sure there isn't already a comment record with the same hash

That file name, line number, and line contents combination seems to be the same keys that GitHub uses to decide whether to collapse comments in the PR interface. But we can tweak the hash components as needed.