Establish src/components/ directory.

Establish src/components/ directory.

TBR=ben@chromium.org
BUG=138280
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=174678

[Jói, 2012-12-19 14:35:20]

https://codereview.chromium.org/11648012/diff/1/components/README
File components/README (right):

https://codereview.chromium.org/11648012/diff/1/components/README#newcode30
components/README:30: The code in each component, or group of components, should
be placed
Note:  The previous decision, when content/components/ was the location for
components, was for their code to live in the content:: namespace.  Not sure
this makes sense any more.

We _could_ specify to use something like c::foo:: for a component named foo,
rather than just foo::.  Or we could require the whole hog, components::foo::,
what do you think?

[jam, 2012-12-19 17:12:58]

lgtm

https://codereview.chromium.org/11648012/diff/1/components/DEPS
File components/DEPS (right):

https://codereview.chromium.org/11648012/diff/1/components/DEPS#newcode4
components/DEPS:4: "-chrome",
nit: technically the two negative lines aren't needed because by default they're
not allowed

https://codereview.chromium.org/11648012/diff/1/components/README
File components/README (right):

https://codereview.chromium.org/11648012/diff/1/components/README#newcode30
components/README:30: The code in each component, or group of components, should
be placed
On 2012/12/19 14:35:20, Jói wrote:
> Note:  The previous decision, when content/components/ was the location for
> components, was for their code to live in the content:: namespace.  Not sure
> this makes sense any more.
> 
> We _could_ specify to use something like c::foo:: for a component named foo,
> rather than just foo::.  Or we could require the whole hog, components::foo::,
> what do you think?

the convention for namespaces has been that every top level directory has a
namespace with that directory's name (with the exception of src\chrome). so that
would imply here that all the code should be in the "components" namespace. you
could add nested namespaces as well, but I think that would make typing very
awkward and with the 80 line limit it would be a lot of indentation.

[Jói, 2012-12-20 15:16:32]

https://codereview.chromium.org/11648012/diff/1/components/DEPS
File components/DEPS (right):

https://codereview.chromium.org/11648012/diff/1/components/DEPS#newcode4
components/DEPS:4: "-chrome",
On 2012/12/19 17:12:58, John Abd-El-Malek wrote:
> nit: technically the two negative lines aren't needed because by default
they're
> not allowed

Right, I just thought the file would be clearer for humans this way :)  Happy to
remove if you think they don't belong.

https://codereview.chromium.org/11648012/diff/1/components/README
File components/README (right):

https://codereview.chromium.org/11648012/diff/1/components/README#newcode30
components/README:30: The code in each component, or group of components, should
be placed
On 2012/12/19 17:12:58, John Abd-El-Malek wrote:
> On 2012/12/19 14:35:20, Jói wrote:
> > Note:  The previous decision, when content/components/ was the location for
> > components, was for their code to live in the content:: namespace.  Not sure
> > this makes sense any more.
> > 
> > We _could_ specify to use something like c::foo:: for a component named foo,
> > rather than just foo::.  Or we could require the whole hog,
components::foo::,
> > what do you think?
> 
> the convention for namespaces has been that every top level directory has a
> namespace with that directory's name (with the exception of src\chrome). so
that
> would imply here that all the code should be in the "components" namespace.
you
> could add nested namespaces as well, but I think that would make typing very
> awkward and with the 80 line limit it would be a lot of indentation.

OK, let's go with components:: as the namespace.  Ben, do you agree?

[Jói, 2012-12-21 10:58:09]

Ben: Would like to get your explicit LGTM.

I uploaded a new patch where I've updated the namespace guidance in the README
file to say "Code in this directory should be placed in the components::
namespace."

Cheers,
Jói


On 2012/12/20 15:16:32, Jói wrote:
> https://codereview.chromium.org/11648012/diff/1/components/DEPS
> File components/DEPS (right):
> 
> https://codereview.chromium.org/11648012/diff/1/components/DEPS#newcode4
> components/DEPS:4: "-chrome",
> On 2012/12/19 17:12:58, John Abd-El-Malek wrote:
> > nit: technically the two negative lines aren't needed because by default
> they're
> > not allowed
> 
> Right, I just thought the file would be clearer for humans this way :)  Happy
to
> remove if you think they don't belong.
> 
> https://codereview.chromium.org/11648012/diff/1/components/README
> File components/README (right):
> 
> https://codereview.chromium.org/11648012/diff/1/components/README#newcode30
> components/README:30: The code in each component, or group of components,
should
> be placed
> On 2012/12/19 17:12:58, John Abd-El-Malek wrote:
> > On 2012/12/19 14:35:20, Jói wrote:
> > > Note:  The previous decision, when content/components/ was the location
for
> > > components, was for their code to live in the content:: namespace.  Not
sure
> > > this makes sense any more.
> > > 
> > > We _could_ specify to use something like c::foo:: for a component named
foo,
> > > rather than just foo::.  Or we could require the whole hog,
> components::foo::,
> > > what do you think?
> > 
> > the convention for namespaces has been that every top level directory has a
> > namespace with that directory's name (with the exception of src\chrome). so
> that
> > would imply here that all the code should be in the "components" namespace.
> you
> > could add nested namespaces as well, but I think that would make typing very
> > awkward and with the 80 line limit it would be a lot of indentation.
> 
> OK, let's go with components:: as the namespace.  Ben, do you agree?

[Jói, 2012-12-21 22:33:26]

ben: One more ping.

I may commit this TBR if I don't hear from you today, and we can sort out any
issues in a follow-up, as kaiwang@ is working on follow-up changes that move the
components in content/components/ to this new directory, and it would be good if
he can land those next week.

Cheers,
Jói

[jam, 2012-12-26 16:04:00]

fyi Ben is OOO for another week. TBRing seems fine since he already gave verbal
agreement about this structure..

On 2012/12/21 22:33:26, Jói wrote:
> ben: One more ping.
> 
> I may commit this TBR if I don't hear from you today, and we can sort out any
> issues in a follow-up, as kaiwang@ is working on follow-up changes that move
the
> components in content/components/ to this new directory, and it would be good
if
> he can land those next week.
> 
> Cheers,
> Jói

[commit-bot: I haz the power, 2012-12-27 09:47:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/11648012/11001

[commit-bot: I haz the power, 2012-12-27 11:44:01]

Change committed as 174678