Learn more about these different git repos.
Other Git URLs
Alice submits a pull request to a repo. Bob finds some issues and posts inline comments. Alice fixes issues and updates the pull request (by force-pushing the same branch). The PR page now shows that Bob commented on line X, but clicking the link display current version of the patch without the comment.
It would be nice if even outdated comments were reachable somehow.
Example: https://pagure.io/rpkg/pull-request/13
Agreed
I got this working by providing a small toggle button allowing to show/hide the comment in the main section of the PR.
This is how it looks like:
<img alt="pagure_show_hide_inline_comment.png" src="/pagure/issue/raw/files/bd5ae1bd780db442ed746c712e8a040f7d9fb1f4d255b08de95c5b58a3691959-pagure_show_hide_inline_comment.png" />
It would probably be useful to also see the line that the comment was about and maybe some context? (or is the "line 10 of folder/readme" a link to the actual commit?
the link is not to the commit but to the comment on the "Diff" tab of the PR page, but that link gets voided if the file changes.
The proposal is good enough for me. If there was a way to display the line the comment was attached to, it would be awesome, but I expect that might not be really possible.
Alternative improvement would be change the text to something "XYZ commented on outdated patch on line 10 of folder/readme so that it's obvious the link does not point to the correct place.
Implemented in https://pagure.io/pagure/pull-request/749
Except that I do not know in front if the files was changed since the comment was made.
Ok then, I thought this might be infeasible.
I'd like this as well but this will take some work and maybe some refactoring, I don't have in mind a way to cleanly extract the line or lines above the comment (especially if the file changed since). Maybe the lines should be stored in the DB together with the comment, but that doesn't sound so nice. Something to think about because it would clearly be good to have :)
As said on IRC: we could instead of the file hash store the tip of the tree (commit at the top) and the file path. This uniquely identifies the state of the file including its changes up to that point.
I'm going to close this one for now.
The discussion for showing the context is at: https://pagure.io/pagure/issue/751
Login to comment on this ticket.