| ngraham | |
| abetts |
| VDG |
If a destination file name is present, it is a better label for a job
than the source file name. This is particularly true for web downloads
where the source file name may be a gibberish base64 string while the
destination file name would be selected by the user.
If there is no destination file name, we fall back to using the source
file name.
BUG: 396744
Compiles, built-in tests pass. Not sure about coverage.
| Lint Skipped |
| Unit Tests Skipped |
Thanks for your patch and explanation on the mailing list!
The idea was mostly to show "which file it is currently copying" but I can see there are more cases where the source is weird than to where the destination is wrong?
I'll add VDG to get some usability/design feedback on that one.
| applets/notifications/package/contents/ui/JobDelegate.qml | ||
|---|---|---|
| 70 | Could be simplified to var label = labelFileName1 || labelFileName0; | |
+1, dest filename makes more sense IMHO. Semantically, the whole point of the move or copy operation is to end up with a destination file, so that's the file that should be focused on--whatever its name is.
Noob question (this is my first patch through phabricator and I don't have commit rights, naturally):
How does one normally proceed now, do I mail or attach here, the patch (as in git format-patch) so someone can git am -s it into the next release's dev branch? Does phabricator do that automatically ("land" it)? Should I create a task for it on some workboard (or is that thinking backwards)?
So now that it's gone through the VDG (Visual Design Group) review, it should go through a code review and final sign-off by the maintainer. Looks like Kai has already done the former, so if he's satisfied now, I'd say it can land! Because you submitted this patch using the web interface, we will need you to provide your real name and email address before landing it for you. Can you provide those in a comment here, please? Nothing further is needed beyond that; we will land it for you if it's ultimately approved by the Plasma developers (which appears likely at this point!).
I see, thanks! Here's my name and email,
Karthik Periagaram
karthik.periagaram@gmail.com