Mark gave review comments related to the build system at bug 714733 comment 87 that haven't been addressed.
Summary: Handle Mark's build-system review comments → Handle Mark's build-system review comments from bug 714733
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.
Component: Instant Messaging → General
Product: Thunderbird → Chat Core
Version: Trunk → trunk
You need to log in before you can comment on or make changes to this bug.