<table><tr><td style="">ngraham added subscribers: bcooksley, ngraham.<br />ngraham added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D16421">View Revision</a></tr></table><br /><div><div><p>Wow, these are truly excellent. I think you've done an amazing job!</p>
<p>One thing I'd like to discuss is whether or not we want the <tt style="background: #ebebeb; font-size: 13px;">emblem-remove</tt> icon to be red. This color is typically reserved for destructive actions and error conditions, and the emblem as far as I can tell is only used in Dolphin--where its usage denotes something that is neither destructive nor an error. Users might be worried that clicking on it will actually remove the item! I wonder if a the Icon Orange color might be more suitable. What do you think?</p>
<p>Another thing is the <tt style="background: #ebebeb; font-size: 13px;">emblem-symbolic-link</tt> icon. It's the only common-ish one that doesn't follow the pattern of having a colored background with a border, which might muddy the design language you've chosen (which I love). Also, I don't think the filled-in background really works: <a href="https://phabricator.kde.org/F6349078" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">F6349078: link icon.png</a></p>
<p>Just throwing out some discussion points, but those are pretty minor and overall this is already a big improvement IMHO.</p>
<hr class="remarkup-hr" />
<p>Since this fixes all three bugs, you can replace</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">https://bugs.kde.org/show_bug.cgi?id=399356
https://bugs.kde.org/show_bug.cgi?id=399357
https://bugs.kde.org/show_bug.cgi?id=399968</pre></div>
<p>with</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">BUG: 399356
BUG: 399357
BUG: 399968
FIXED-IN: 5.52</pre></div>
<hr class="remarkup-hr" />
<p>Unfortunately, the patch does not apply cleanly, and I don't think it's your fault:</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">arc patch D16421
[...]
Checking patch icons/emblems/22/emblem-pause.svg...
Checking patch icons/emblems/22/emblem-mounted.svg...
Checking patch dev/null => icons/emblems/22/emblem-locked.svg...
error: dev/null: does not exist in index
Checking patch icons/emblems/22/emblem-information.svg...
Checking patch icons/emblems/22/emblem-important.svg...
Checking patch dev/null => icons/emblems/22/emblem-favorite.svg...
error: dev/null: does not exist in index
Checking patch icons/emblems/22/emblem-error.svg...
Checking patch icons/emblems/22/emblem-encrypted-unlocked.svg...
[...]
Checking patch icons-dark/emblems/22/emblem-mounted.svg...
Checking patch dev/null => icons-dark/emblems/22/emblem-locked.svg...
error: dev/null: does not exist in index
Checking patch icons-dark/emblems/22/emblem-information.svg...
Checking patch icons-dark/emblems/22/emblem-important.svg...
Checking patch dev/null => icons-dark/emblems/22/emblem-favorite.svg...
error: dev/null: does not exist in index
Checking patch icons-dark/emblems/22/emblem-error.svg...
Checking patch icons-dark/emblems/22/emblem-encrypted-unlocked.svg...</pre></div>
<p>What's going on here is that some symlinks are being replaced with new files, and other files are being replaced with symlinks. <tt style="background: #ebebeb; font-size: 13px;">arc</tt> doesn't seem too happy about this. <a href="https://phabricator.kde.org/p/bcooksley/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@bcooksley</a> or anyone else from <a href="https://phabricator.kde.org/tag/sysadmin/" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">#sysadmin</a>, any idea what to do here?</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R266 Breeze Icons</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D16421">https://phabricator.kde.org/D16421</a></div></div><br /><div><strong>To: </strong>ndavis, VDG<br /><strong>Cc: </strong>ngraham, bcooksley, kde-frameworks-devel, VDG, michaelh, bruns<br /></div>