1231315 - Build CONFIGURE_DEFINE_FILES at build time instead of during configure

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Build CONFIGURE_DEFINE_FILES at build time instead of during configure

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8696936 [details] [diff] [review]
Build CONFIGURE_DEFINE_FILES at build time instead of during configure

Review of attachment 8696936 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/action/process_define_files.py
@@ +29,5 @@
> +    with open(path, 'rU') as input:
> +        r = re.compile('^\s*#\s*(?P<cmd>[a-z]+)(?:\s+(?P<name>\S+)(?:\s+(?P<value>\S+))?)?', re.U)
> +        for l in input:
> +            m = r.match(l)
> +            if m:

The indent pyramid is hurting my eyes. Can you invert this and the "if name" below and use "continue."

Comment 3

[Mike Hommey [:glandium]]

(In reply to Gregory Szorc [:gps] from comment #2)
> Comment on attachment 8696936 [details] [diff] [review]
> Build CONFIGURE_DEFINE_FILES at build time instead of during configure
> 
> Review of attachment 8696936 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: python/mozbuild/mozbuild/action/process_define_files.py
> @@ +29,5 @@
> > +    with open(path, 'rU') as input:
> > +        r = re.compile('^\s*#\s*(?P<cmd>[a-z]+)(?:\s+(?P<name>\S+)(?:\s+(?P<value>\S+))?)?', re.U)
> > +        for l in input:
> > +            m = r.match(l)
> > +            if m:
> 
> The indent pyramid is hurting my eyes. Can you invert this and the "if name"
> below and use "continue."

I'm pretty sure you asked the same question when the same code was reviewed at the location I'm moving it from. And the problem is still the same: there's something happening after that if.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/75e62a17dbba

Comment 5

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #3)
> > The indent pyramid is hurting my eyes. Can you invert this and the "if name"
> > below and use "continue."
> 
> I'm pretty sure you asked the same question when the same code was reviewed
> at the location I'm moving it from. And the problem is still the same:
> there's something happening after that if.

Landed as is because the code is unchanged from what was already in the tree. If you feel strongly that this should be improved, please file a followup.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/75e62a17dbba