Last modified: 2013-06-27 14:52:32 UTC
Gerrit doesn't just not include effective tools for reviewing/diffing images and other media, it also makes it inordinately hard to view old and new versions by putting them in archives. If there is no configuration that can be changed to make it so images and whatnot can be displayed inline or at very least directly downloaded, I propose we raise a mob and burn down upstream because this is just ridiculous.
Hm, they used to be displayed in side-by-side diff a few months ago at least, but they are no more. Live example: https://gerrit.wikimedia.org/r/#/c/54136/
There *is* a direct download link on the diff view -- it's the funky icon with the blue arrow to the right of the patch revision numbers. Yeah, easy to discover eh? Could... use some love. Also this doesn't work well for 'odd' file types such as the .ico files generally used for favicons. (See eg <https://gerrit.wikimedia.org/r/#/c/60338/1/docroot/bits/favicon/wikidata.ico> where it offers a *zip file* of the .ico download.) Personally I just check out the change in a local repo copy and look at the file manually. :P
This definitely used to work.
(In reply to comment #2) > Also this doesn't work well for 'odd' file types such as the .ico files .ico files were recognized as application/octet-stream. Hence, there were not shown. Pushed a fix for the .ico problem to: https://gerrit-review.googlesource.com/#/c/45010/
As for the general problem of showing images in diffs: * For unified diffs, the change in ustream is at https://gerrit-review.googlesource.com/#/c/45000/ * For side-by-side diffs, this seems to have never been implemented. Now it is at https://gerrit-review.googlesource.com/#/c/45007/
(In reply to comment #5) > As for the general problem of showing images in diffs: > * For unified diffs, the change in ustream is at > https://gerrit-review.googlesource.com/#/c/45000/ > * For side-by-side diffs, this seems to have never been implemented. > https://gerrit-review.googlesource.com/#/c/45007/ Christian: Both upstream patches have been merged, are there plans to backport/deploy to the WM Gerrit instance (or has this happened already)?
We're working on putting together a new build, these patches will be included in that.
This has been fixed in our latest build we deployed, 2.7-rc2-505-g7502a46.
