Doesn't really work for PRs on branches with unsquashed commits

[rmloveland]

If a PR has several commits on the branch, re shows a diff that does not reflect all of the changes that make up the PR, but only a subset (the latest commit?).

This may be how git works and have nothing to do with re. But if there is a way for re to work in this instance, that would be cool.

Otherwise I volunteer to update the README to note that you must be working with PRs on branches that have all their commits squashed (which seems like the right way to live anyway - just surprised me in this case).

[jordanlewis]

Hmm - that doesn't seem right. Can you give me an example of a PR that has this problem? re was designed specifically for multi-commit PRs.

[rmloveland]

Probably too late to be useful now, but it was this docs PR. I ended up asking Lauren to squash so I could do the review with re (which worked fine at that point).

I did find the re-edit-* file from the earlier attempt. I'm attaching it to this comment. Hopefully it shows you what was happening (it's possible this is a PEBKAC situation).

re-edit-201726569.txt

If that is not helpful I will set up another PR to try to repro.

[jordanlewis]

Ah - yes! That file actually seems correct to me. `re` presents a review as one set of diffs per commit. If you scroll through that file, you can see that all of the diffs are eventually available for each file - they're just not squashed for you. This allows you to leave comments on diffs from multiple different commits, which facilitates a mode of PR-review where people are trying to PR a sequence of fully-baked commits at once. The current mode of operation of `re` isn't set up to facilitate review of PRs that have multiple "fixup" commits stacked onto each other - for that you do want a squash mode of review, which I haven't implemented yet. But perhaps we can add that as an option - I don't think it's too hard to look at the full-PR diff from GitHub's API instead of the per-commit diff. In fact, implementing the per-commit diff was one of the more difficult and annoying tasks of implementing `re`.

…

On Mon, Sep 10, 2018 at 5:27 PM Rich Loveland ***@***.***> wrote: Probably too late to be useful now, but it was this docs PR <cockroachdb/docs#3669>. I ended up asking Lauren to squash so I could do the review with re (which worked fine at that point). I did find the re-edit-* file from the earlier attempt. I'm attaching it to this comment. Hopefully it shows you what was happening (it's possible this is a PEBKAC situation). re-edit-201726569.txt <https://github.com/jordanlewis/re/files/2368452/re-edit-201726569.txt> If that is not helpful I will set up another PR to try to repro. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACrLVLUiFRsu1qGrIEcJMbLTK5P9qcIks5uZtlZgaJpZM4WiKgG> .

[rmloveland]

This issue appears to have been user confusion on my part and can be closed IMO. Just filed a feature request for the "squash mode" in #5.