735723 - Handle Mark's build-system review comments from bug 714733

Status: NEW

(Chat Core :: General, defect)

Description

[Florian Quèze [:florian]]

Mark gave review comments related to the build system at bug 714733 comment 87 that haven't been addressed.

Comment 1

[Florian Quèze [:florian]]

bug 714733 comment 87 is long and contains lots of l10n-related points that were addressed a long time ago, so I'm going to paste here what hasn't been addressed yet:

(Mark Banner (:standard8) in bug 714733 comment #87)
> Comment on attachment 604802 [details] [diff] [review]

> I think the Build Config changes are largely performance improvements for
> building, and possibly for TB itself, and so may be fixed in a follow-up bug
> post landing.
> 
> ::: chat/Makefile.in
> @@ +41,5 @@
> > +
> > +include $(DEPTH)/config/autoconf.mk
> > +
> > +PROTOCOLS = \
> > +		facebook \
> 
> nit: we've moved to using two-space indent rather than tabs for these type
> of continuations in Makefiles.
> 
> @@ +56,5 @@
> > +PREF_JS_EXPORTS = $(srcdir)/chat-prefs.js
> > +
> > +PARALLEL_DIRS	= \
> > +		components/public \
> > +		components/src \
> 
> Please consider putting these into one directory, maybe mixed with modules
> as well. We're generally trying to cut down on the amount of separate
> directories that make has to enter.
> 
> @@ +61,5 @@
> > +		modules \
> > +		content \
> > +		themes \
> > +		locales \
> > +		$(foreach proto,$(PROTOCOLS),protocols/$(proto)) \
> 
> Could we restructure this so that we only have one Makefile in protocols,
> and everything in the sub-directories is included from that? I'd really
> prefer not to have a multitude of directories added.
> 
> ::: chat/components/src/Makefile.in
> @@ +41,5 @@
> > +
> > +include $(DEPTH)/config/autoconf.mk
> > +
> > +EXTRA_COMPONENTS = \
> > +		imAccounts.js imAccounts.manifest \
> 
> Unless you've got a good reason to do so, I'd suggest incorporating all the
> .manifest files into one, this will help avoid individual file processing
> during the make and packaging stages. Possibly during startup as well,
> though I can't quite remember how much they are already grouped together.