Project

General

Profile

Feature #5615

Misleading message in PR diff view "File was deleted in this version"

Added by Olivier Renaud 4 months ago. Updated 4 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
15.04.2020
Due date:
% Done:

0%

Estimated time:
Sorting:
Commit Number:

Description

I have a PR with an inline code comment. I updated the PR with a new commit that reverts the code that the comment was attached to. Now, the PR page correctly keeps the inline comment in a dedicated block at the end of the diffs. My problem is that the message that appears in this block is misleading.

It says that the file was deleted, but it's not the case: there is just no change in this file anymore, in the complete diff.

2020-04-15_19-56-40.png (7.63 KB) 2020-04-15_19-56-40.png Olivier Renaud, 15.04.2020 20:01
Screen Shot 2020-04-22 at 11.48.07.png (39.9 KB) Screen Shot 2020-04-22 at 11.48.07.png Daniel D, 22.04.2020 11:52
3070
3071

History

#1 Updated by Daniel D 4 months ago

Hi Oliver,

This is somehow expected. Our system tracks changes of pull-requests, and there were actions attached to the mentioned file, that was now deleted in the next iteration of a pull-request. This is why we display this information. If you would go back to the previous version we'd still show that file.

Would it help if we work on the message and explain it more why we mark it as deleted in this version ?

#2 Updated by Olivier Renaud 4 months ago

I can see how technically a version of a PR "deleted" a file compared to a previous version. But the word "delete" really suggests that my PR includes the deletion of the file (as in hg remove or git rm).

I can suggest:
removed in this version -> no changes in this version
File was deleted in this version -> File does not contain changes anymore in this version

#3 Updated by Daniel D 4 months ago

  • Target version set to v4.19
  • Assignee set to Daniel D

Thanks for the suggestion, you're right "deleted" really is too strong, and indicates permanent action.

We'll discuss this internally, and adjust the names for that case operation.

#4 Updated by Daniel D 4 months ago

3071

This is our current proposal, let us know what do you think?

#5 Updated by Olivier Renaud 4 months ago

I'm fine with this new message, yes. It's certainly less confusing than the current one.
Thanks.

#6 Updated by Redmine Integration 4 months ago

  • Status changed from New to Resolved

Also available in: Atom PDF