Issue metadata
Sign in to add a comment
|
Match added files that were renamed between patch sets to ease diff-ing |
||||||||||||||||||||||||
Issue descriptionIn this CL [1] some newly introduced files were renamed between patch sets #8 and #9: chrome/browser/android/offline_pages/suggestions_observer.(h|cc) became suggested_articles_observer.(h|cc) . I had comments in patch set #7 that I wanted to compare to the changes introduced in #9, but as the renamed files are completely dissociated from its previous versions they show up as new files without past history in the change list [2], making its reviewing process more tedious. It would be better if we could do content matching for added files between patch sets so that they could be directly diff-ed through renames. [1] https://codereview.chromium.org/2811813002 [2] https://codereview.chromium.org/2811813002/diff2/180001:220001/chrome/browser/android/offline_pages/suggested_articles_observer.cc
,
Jun 6 2017
I now started using Gerrit and noticed the same problem applies for it. In this diff view: https://chromium-review.googlesource.com/c/523282/2..7 There were many files that were renamed between patch sets 2 and 7 but they show up as deleted and added, making the review more difficult. For instance: offline_metrics_collector_stub.h was renamed to test_offline_metrics_collector.h.
,
Jun 6 2017
,
Jun 6 2017
Yes, this is a thing we'd love to do better at. Obviously, Rietveld is super bad at this (actually worse than Gerrit, due to differences in the way patchfiles are generated vs commit objects), so this isn't a priority for us, but it's something we want to look into. The solution will almost certainly involve server-side settings for the 'git log'/'git diff' rename threshold.
,
Jan 3 2018
Removing Milestone-Afterglow, as it has ceased to have meaning. More refined milestones may be added back in the near future.
,
Jan 3 2018
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by andyb...@chromium.org
, Apr 18 2017Status: Available (was: Untriaged)