Rename app_modal_dialogs dir to app_modal

Rename app_modal_dialogs dir to app_modal

BUG=410499
TBR=benwells@chromium.org, jochen@chromium.org, agl@chromium.org

Committed: https://crrev.com/f6539842bf88bd2858183936277f9d3a43ac9093
Cr-Commit-Position: refs/heads/master@{#304697}

[oshima, 2014-11-17 18:57:45]

Patchset #1 (id:1) has been deleted

[oshima, 2014-11-17 18:58:31]

oshima@chromium.org changed reviewers:
+ avi@chromium.org, msw@chromium.org

[msw, 2014-11-17 19:14:41]

lgtm with nits

https://codereview.chromium.org/735473002/diff/20001/components/app_modal.gypi
File components/app_modal.gypi (left):

https://codereview.chromium.org/735473002/diff/20001/components/app_modal.gyp...
components/app_modal.gypi:23: 'app_modal_dialogs/app_modal_dialog_test_util.h',
Good catch... I wonder how this file got deleted but not these entries.

https://codereview.chromium.org/735473002/diff/20001/components/app_modal/jav...
File components/app_modal/javascript_dialog_manager_impl.h (right):

https://codereview.chromium.org/735473002/diff/20001/components/app_modal/jav...
components/app_modal/javascript_dialog_manager_impl.h:4: #ifndef
COMPONENTS_APP_MODAL_JAVASCRIPT_DIALOG_MANAGER_IMPL_H_
nit: blank line above.

https://codereview.chromium.org/735473002/diff/20001/components/app_modal/jav...
File components/app_modal/javascript_native_dialog_factory.h (right):

https://codereview.chromium.org/735473002/diff/20001/components/app_modal/jav...
components/app_modal/javascript_native_dialog_factory.h:8: 
nit: remove one of these blank lines

[oshima, 2014-11-17 21:40:23]

https://codereview.chromium.org/735473002/diff/20001/components/app_modal.gypi
File components/app_modal.gypi (left):

https://codereview.chromium.org/735473002/diff/20001/components/app_modal.gyp...
components/app_modal.gypi:23: 'app_modal_dialogs/app_modal_dialog_test_util.h',
On 2014/11/17 19:14:41, msw wrote:
> Good catch... I wonder how this file got deleted but not these entries.

AFAIK, build system doesn't check the existence of file
unless it's required to build stuff (like .cc).

It'd be nice to have presubmit warning though.

https://codereview.chromium.org/735473002/diff/20001/components/app_modal/jav...
File components/app_modal/javascript_dialog_manager_impl.h (right):

https://codereview.chromium.org/735473002/diff/20001/components/app_modal/jav...
components/app_modal/javascript_dialog_manager_impl.h:4: #ifndef
COMPONENTS_APP_MODAL_JAVASCRIPT_DIALOG_MANAGER_IMPL_H_
On 2014/11/17 19:14:41, msw wrote:
> nit: blank line above.

Done.

https://codereview.chromium.org/735473002/diff/20001/components/app_modal/jav...
File components/app_modal/javascript_native_dialog_factory.h (right):

https://codereview.chromium.org/735473002/diff/20001/components/app_modal/jav...
components/app_modal/javascript_native_dialog_factory.h:8: 
On 2014/11/17 19:14:41, msw wrote:
> nit: remove one of these blank lines

Done.

[Avi (use Gerrit), 2014-11-18 16:21:29]

I'm a bit cold about the name "app modal"; it's an adjective. App-modal _what_?
But on the other hand, I don't really care much any more. I'd much prefer a name
with "dialog" in there, because that's the whole point of this component, that
it provides dialogs, but fatigue has set in on this set of changes.

[oshima, 2014-11-18 17:42:48]

On 2014/11/18 16:21:29, Avi wrote:
> I'm a bit cold about the name "app modal"; it's an adjective. App-modal
_what_?

Mike suggested this because there is already components/web_modal, which
implements web modal dialogs.

> But on the other hand, I don't really care much any more. I'd much prefer a
name
> with "dialog" in there, because that's the whole point of this component, that
> it provides dialogs, but fatigue has set in on this set of changes.

I really have no opinion about naming (unless it's really bad, of course).
This is largely the matter of owner's preference, and I'm happy to abandon
this if you prefer app_modal_dialogs.

Can you review https://codereview.chromium.org/688853005/ then?

I'll create a separate CL to cleanup errors I found in this CL.

[Avi (use Gerrit), 2014-11-18 18:17:16]

On 2014/11/18 17:42:48, oshima wrote:
> On 2014/11/18 16:21:29, Avi wrote:
> > I'm a bit cold about the name "app modal"; it's an adjective. App-modal
> _what_?
> 
> Mike suggested this because there is already components/web_modal, which
> implements web modal dialogs.
> 
> > But on the other hand, I don't really care much any more. I'd much prefer a
> name
> > with "dialog" in there, because that's the whole point of this component,
that
> > it provides dialogs, but fatigue has set in on this set of changes.
> 
> I really have no opinion about naming (unless it's really bad, of course).
> This is largely the matter of owner's preference, and I'm happy to abandon
> this if you prefer app_modal_dialogs.
> 
> Can you review https://codereview.chromium.org/688853005/ then?
> 
> I'll create a separate CL to cleanup errors I found in this CL.

LGTM.

[Avi (use Gerrit), 2014-11-18 21:50:15]

To clarify, if we already have a web_modal components, then I guess it makes
sense-ish to have app_modal. I'm OK with that naming. I'd prefer both to refer
to "dialog", but not strongly enough to withhold any approval over what would be
bikeshedding.

[oshima, 2014-11-18 22:00:51]

oshima@chromium.org changed reviewers:
+ benwells@chromium.org, jochen@chromium.org

[oshima, 2014-11-18 22:01:36]

tbr'ing benwells@chromium.org and jochen@chromium.org for trival renaming of the
directory of DEPS file.

[oshima, 2014-11-18 22:02:37]

oshima@chromium.org changed reviewers:
+ willchan@chromium.org

[oshima, 2014-11-18 22:03:11]

oshima@chromium.org changed reviewers:
+ agl@chromium.org
- willchan@chromium.org

[oshima, 2014-11-18 22:03:40]

and agl@ for net/base in moved DEPS file.

[oshima, 2014-11-18 22:03:46]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-11-18 22:05:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/735473002/60001

[commit-bot: I haz the power, 2014-11-18 23:31:11]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2014-11-18 23:32:02]

Patchset 3 (id:??) landed as
https://crrev.com/f6539842bf88bd2858183936277f9d3a43ac9093
Cr-Commit-Position: refs/heads/master@{#304697}

[oshima, 2014-11-18 23:59:02]

Patchset #5 (id:100001) has been deleted

[oshima, 2014-11-18 23:59:11]

Patchset #4 (id:80001) has been deleted