Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(143)

Issue 10206015: 1. Fix JSDocs in directory_model.js. (Closed)

Created:
8 years, 8 months ago by Oleg Eterevsky
Modified:
8 years, 8 months ago
CC:
chromium-reviews, rginda+watch_chromium.org, arv (Not doing code reviews)
Visibility:
Public.

Description

1. Fix JSDocs in directory_model.js. 2. Replace all getters and setters in directory_model.js with get*() and set*() methods. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=133663

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+252 lines, -205 lines) Patch
M chrome/browser/resources/file_manager/js/directory_model.js View 27 chunks +198 lines, -157 lines 2 comments Download
M chrome/browser/resources/file_manager/js/file_manager.js View 34 chunks +44 lines, -43 lines 0 comments Download
M chrome/browser/resources/file_manager/js/file_manager_pyauto.js View 3 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/resources/file_manager/js/file_transfer_controller.js View 2 chunks +6 lines, -3 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Oleg Eterevsky
8 years, 8 months ago (2012-04-24 09:50:23 UTC) #1
dgozman
Changes LGTM. However: - I cannot check that all usages are changed - there are ...
8 years, 8 months ago (2012-04-24 10:58:13 UTC) #2
Oleg Eterevsky
8 years, 8 months ago (2012-04-24 11:03:32 UTC) #3
Thanks for the review.

I've tried my best to check that I change all the uses of these fields. Also the
FileManager with these changes is running fine.

I'm not sure I'm the best person to write the method comments here, since I do
not know what some of them actually do.

https://chromiumcodereview.appspot.com/10206015/diff/1/chrome/browser/resourc...
File chrome/browser/resources/file_manager/js/directory_model.js (right):

https://chromiumcodereview.appspot.com/10206015/diff/1/chrome/browser/resourc...
chrome/browser/resources/file_manager/js/directory_model.js:1182: * @param
{string} path Path.
On 2012/04/24 10:58:14, dgozman wrote:
> 'Any path' for consistency? :)

Changed to "A path".

Powered by Google App Engine
This is Rietveld 408576698