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
•