Closed
Bug 290490
Opened 20 years ago
Closed 20 years ago
integrate suite/ into build process
Categories
(SeaMonkey :: Build Config, defect)
SeaMonkey
Build Config
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: kairo, Assigned: kairo)
Details
Attachments
(1 file, 1 obsolete file)
8.07 KB,
patch
|
benjamin
:
review+
neil
:
superreview+
asa
:
approval1.8b2+
|
Details | Diff | Splinter Review |
Now that the suite/ module is there, we should integrate it into the build
process and pull it.
Patch coming up in a moment.
![]() |
Assignee | |
Comment 1•20 years ago
|
||
OK, I hope this patch does what we want. I also introduced a MOZ_SEAMONKEY
define for including suite-specific stuff, so we can also define MOZ_XUL_APP
one time and don't get bitten by suddely excluding our stuff ;-)
Attachment #180815 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #180815 -
Flags: review?(chase)
Comment 2•20 years ago
|
||
maybe the define should be MOZ_SUITE instead?
![]() |
Assignee | |
Comment 3•20 years ago
|
||
(In reply to comment #2)
> maybe the define should be MOZ_SUITE instead?
Actually, I have no feelings about that, I just followed the example of the
other apps we have... Not sure what is The Right Thing (tm) to do here...
Comment 4•20 years ago
|
||
Comment on attachment 180815 [details] [diff] [review]
patch: add suite/ to build
Since the toolkit apps do not wish to pull suite/, we need to fix the
MODULES_toolkit stuff. Probably have a MODULES_core and make MODULES_suite
extend that.
In addition, we should not be coding MOZ_SEAMONKEY. Try MOZ_SUITE instead?
Lastly, adding MOZ_SEAMONKEY=1 to configure.in doesn't help at all unless you
AC_DEFINE it.
Attachment #180815 -
Flags: review?(chase) → review-
Comment 5•20 years ago
|
||
I would personally prefer (as well) to keep the generic (Suite) where possible
rather than a product/project name. Just makes more sense to me, though I am
not sure if that will conflict with anything. And I will accept whatever the
final choice is regardless.
![]() |
Assignee | |
Comment 6•20 years ago
|
||
OK, this changes to MOZ_SUITE, and introduces MODULES_core to client.mk (which
is exactly what MOZDULES_suite was until now), so that we don't have to include
the whole suite into toolkit et al.
The AC_DEFINE is now there as well, thanks for spotting it :)
![]() |
Assignee | |
Updated•20 years ago
|
Attachment #180815 -
Attachment is obsolete: true
Attachment #180824 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #180824 -
Flags: review?(benjamin)
Comment 7•20 years ago
|
||
Comment on attachment 180815 [details] [diff] [review]
patch: add suite/ to build
>+DIRS =
>+
Shouldn't this be
DIRS = \
$(NULL)
so as to avoid having to modify that line in future?
moa=me with this and whatever it is bsmedberg wants fixed.
Attachment #180815 -
Attachment is obsolete: false
Attachment #180815 -
Flags: superreview?(neil.parkwaycc.co.uk)
Comment 8•20 years ago
|
||
Comment on attachment 180824 [details] [diff] [review]
patch v2: address bsmedberg's comments
Are you planning on landing this patch before you've got anything more in
mozilla/suite ? If so, you should remove the DIRS= line in that file until you
actually have subdirectories to build.
Attachment #180824 -
Flags: review?(benjamin) → review+
Updated•20 years ago
|
Attachment #180824 -
Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
![]() |
Assignee | |
Comment 9•20 years ago
|
||
(In reply to comment #8)
> (From update of attachment 180824 [details] [diff] [review] [edit])
> Are you planning on landing this patch before you've got anything more in
> mozilla/suite ? If so, you should remove the DIRS= line in that file until you
> actually have subdirectories to build.
OK, good.
Yes, I'm actually planning to land it so that someone putting real stuff in
suite/ (e.g. gandalf with locales, or anyone working on the toolkit installer -
our two first things targeting suite/) doesn't need to care about how to get the
dir itself into the build process, but only has to care about what's happening
in suite/ itself.
It doesn't harm us to step in and build nothing right now, but it eases the
process for someone who puts stuff in there.
![]() |
Assignee | |
Updated•20 years ago
|
Attachment #180815 -
Attachment is obsolete: true
![]() |
Assignee | |
Comment 10•20 years ago
|
||
Comment on attachment 180824 [details] [diff] [review]
patch v2: address bsmedberg's comments
requesting approval: this adds the new, but still empty suite/ dir to the suite
build process only.
Attachment #180824 -
Flags: approval1.8b2?
Comment 11•20 years ago
|
||
Comment on attachment 180824 [details] [diff] [review]
patch v2: address bsmedberg's comments
a=asa
Attachment #180824 -
Flags: approval1.8b2? → approval1.8b2+
![]() |
Assignee | |
Comment 12•20 years ago
|
||
checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 13•20 years ago
|
||
Just a note, the checkin reading
bug 290490 - editorOverlay inserts a second menupopup into message compose view
show/hide menu, patch by Justin Wood <bugspam.Callek@gmail.com> r+sr=Neil a=asa
Should really point to Bug 289366. Marking here so blame can still be useful
for those changes.
Verified FIXED; I build on Windows XP all the time with no problem.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•