|
|
Created:
6 years, 4 months ago by hirono Modified:
6 years, 4 months ago Reviewers:
mtomasz CC:
chromium-reviews, rginda+watch_chromium.org, mtomasz+watch_chromium.org, yoshiki+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Project:
chromium Visibility:
Public. |
DescriptionFiles.app: Fix nits in FileOperationManager.
* Assign correct entry for entry-change event.
* Assign totalBytes to processedBytes at the end of zipping.
* Determine if entries are movable or not by comparing file system.
BUG=None
TEST=manually
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289291
Patch Set 1 #
Total comments: 5
Patch Set 2 : Fixed. #Patch Set 3 : Remove one assertion. #
Messages
Total messages: 20 (0 generated)
PTAL the CL? Thank you!
https://codereview.chromium.org/466583003/diff/1/ui/file_manager/file_manager... File ui/file_manager/file_manager/background/js/file_operation_manager.js (right): https://codereview.chromium.org/466583003/diff/1/ui/file_manager/file_manager... ui/file_manager/file_manager/background/js/file_operation_manager.js:244: (source.isFile ? parent.getFile : parent.getDirectory).call( Why the previous code didn't work?
https://codereview.chromium.org/466583003/diff/1/ui/file_manager/file_manager... File ui/file_manager/file_manager/background/js/file_operation_manager.js (right): https://codereview.chromium.org/466583003/diff/1/ui/file_manager/file_manager... ui/file_manager/file_manager/background/js/file_operation_manager.js:244: (source.isFile ? parent.getFile : parent.getDirectory).call( On 2014/08/12 09:35:04, mtomasz wrote: > Why the previous code didn't work? The destinationUrl pointed the destination directory containing the copied entry. But we would like to point the copied entry here.
lgtm https://codereview.chromium.org/466583003/diff/1/ui/file_manager/file_manager... File ui/file_manager/file_manager/background/js/file_operation_manager.js (right): https://codereview.chromium.org/466583003/diff/1/ui/file_manager/file_manager... ui/file_manager/file_manager/background/js/file_operation_manager.js:244: (source.isFile ? parent.getFile : parent.getDirectory).call( On 2014/08/12 09:45:22, hirono wrote: > On 2014/08/12 09:35:04, mtomasz wrote: > > Why the previous code didn't work? > > The destinationUrl pointed the destination directory containing the copied > entry. But we would like to point the copied entry here. Got it. https://codereview.chromium.org/466583003/diff/1/ui/file_manager/file_manager... ui/file_manager/file_manager/background/js/file_operation_manager.js:247: null , nit: Can we remove this "null"? This argument seems optional.
Thank you! https://codereview.chromium.org/466583003/diff/1/ui/file_manager/file_manager... File ui/file_manager/file_manager/background/js/file_operation_manager.js (right): https://codereview.chromium.org/466583003/diff/1/ui/file_manager/file_manager... ui/file_manager/file_manager/background/js/file_operation_manager.js:247: null , On 2014/08/13 01:05:55, mtomasz wrote: > nit: Can we remove this "null"? This argument seems optional. I tried it but we cannot. Maybe we can drop it only when we does not specify callbacks.
The CQ bit was checked by hirono@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hirono@chromium.org/466583003/20001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
@mtomasz - I removed one assertion from the test. The assertion is not passed due to lack of mock functionality. I bring back the assertion by crrev.com/460963002. Thank you!
The CQ bit was checked by hirono@chromium.org
still lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hirono@chromium.org/466583003/40001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_chromium_gn_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) win_chromium_x64_rel on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_gn_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by hirono@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hirono@chromium.org/466583003/40001
Message was sent while issue was closed.
Committed patchset #3 (40001) as 289291 |