Closed
Bug 285653
Opened 19 years ago
Closed 19 years ago
add reporter tool to nightly builds
Categories
(Other Applications Graveyard :: Reporter, defect)
Other Applications Graveyard
Reporter
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: jay, Assigned: raccettura)
References
Details
Attachments
(8 files, 7 obsolete files)
13.04 KB,
patch
|
raccettura
:
review+
bryner
:
superreview+
|
Details | Diff | Splinter Review |
1.86 KB,
patch
|
Details | Diff | Splinter Review | |
24.80 KB,
patch
|
benjamin
:
review+
asa
:
approval1.8b2+
|
Details | Diff | Splinter Review |
1.36 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
5.46 KB,
patch
|
asa
:
approval-l10n+
asa
:
approval-aviary1.1a1+
|
Details | Diff | Splinter Review |
2.11 KB,
patch
|
benjamin
:
review+
asa
:
approval1.8b2+
|
Details | Diff | Splinter Review |
5.16 KB,
patch
|
asa
:
approval1.8b2+
|
Details | Diff | Splinter Review |
946 bytes,
patch
|
chase
:
review+
asa
:
approval-aviary1.1a1+
asa
:
approval1.8b2+
|
Details | Diff | Splinter Review |
Just like dom inspector, we want to enable the reporter tool in nightly builds. We probably want to disable it for release builds, but we can figure out how to do that later. For now, I'm just going to do what bryner has outlined and see if I can get it working. From my understanding, this is what needs to happen: 1. Get extensions/reporter hooked up to the build system - Create Makefile.in and jar.mn to package up reporter.jar - Add entry for reporter in mozilla/allmakefiles.sh 2. Make necessary changes in browser/installer/windows and /unix - Create new reporter.jst files - Add reporter.jar to packages-static - Add reporter to ComponentList in installer.cfg - Add new reporter section in config.it I'm completely new to this, so that is all I have for now. If I'm missing something, please let me know. I'm going to attach a rough patch soon.
Assignee | ||
Comment 1•19 years ago
|
||
Sounds good to me. Just to clarify: if changes are made within the reporter component (data collection, transmission, etc), I want to + anything on that level. Things regarding build, file organization etc... no problem. Just want to make sure there are no unexpected changes regarding data. Other than that... go for the gold. Regarding disabling in release builds: should of course consider keeping it, so we can tell end users to keep out of bugzilla and notify us through this and talkback. That's another battle of course.
Comment 2•19 years ago
|
||
You rock Jay! (Hey, I have to be able to express useless-but-supporting Bugzilla comments somewhere, right?)
Assignee | ||
Comment 3•19 years ago
|
||
+1 /sorry for bug spam
Reporter | ||
Comment 4•19 years ago
|
||
The changes from my original comment have been made and I was able to run my build and see the reporter item in the help menu. I could not get the id from the server, so we might need to look into that.
Attachment #177084 -
Flags: superreview?(bryner)
Attachment #177084 -
Flags: review?(robert)
Assignee | ||
Comment 5•19 years ago
|
||
Server is busted at the moment after some config changes justdave made a few weeks ago. I haven't bothered to fix it since one of the things apparantly effected, I'm going to be replacing soon (new DB abstraction class)... see bug 285536. I'm planning to update the reporter-test instance at some point early next week (if not sooner).
Comment 6•19 years ago
|
||
Comment on attachment 177084 [details] [diff] [review] patch to get reporter working in nightlies --- browser/installer/unix/config.it 8 Mar 2005 06:39:29 -0000 1.8 +++ browser/installer/unix/config.it 11 Mar 2005 02:12:15 -0000 @@ -631,6 +633,16 @@ Force Upgrade File0=[SETUP PATH]\chrome\@AB_CD@.jar FileCount=$FileCount$ +[Component RPT] +Description Short=@RPT_SHORT@ +Description Long=@RPT_SHORT@ +Archive=reporter.xpi +$InstallSize$:reporter +$InstallSizeArchive$:reporter.xpi +Attributes=SELECTED|VISIBLE|FORCE_UPGRADE +Force Upgrade File0=[SETUP PATH]\chrome\reporter.jar The unix installer files need forward slashes in the path. >--- browser/installer/unix/packages-static 24 Feb 2005 20:11:03 -0000 1.28 >+++ browser/installer/unix/packages-static 11 Mar 2005 02:12:15 -0000 >@@ -314,3 +314,6 @@ > bin/components/libqfaservices.so > bin/components/qfaservices.xpt > bin/components/talkback/* >+ >+[reporter] >+bin\chrome\reporter.jar Same here. Looks ok otherwise, sr=me with those changes.
Attachment #177084 -
Flags: superreview?(bryner) → superreview+
Assignee | ||
Comment 7•19 years ago
|
||
Comment on attachment 177084 [details] [diff] [review] patch to get reporter working in nightlies Oh yea. r=me I'm not reviewing the actual build files. Only that it doesn't harm the system as a whole (that's all I'm comfortable doing). if you want a better r, feel free to ask someone else. I'm only concered with data being fubar'd. I trust bryner's experience.
Attachment #177084 -
Flags: review?(robert) → review+
Assignee | ||
Comment 8•19 years ago
|
||
question for bryner (relevent to this bug). Want to store the reporter URL (currently: http://reporter-test.mozilla.org/service/) in the prefs, rather than hardcoded, so it's like update, and other services. How would that work with the build system? Would we just hard code it in place (so regardless of if reporter is enabled it goes in? Anyway... before this lands, this should be in the prefs if possible: extension.reporter.url = http://reporter-test.mozilla.org/service I'll come up with the appropriate reporter patch.
Reporter | ||
Comment 9•19 years ago
|
||
Robert: Any update on the prefs change? I'm ready to check in my patch...so just give me the word. Thanks.
Assignee | ||
Comment 10•19 years ago
|
||
A few notes here (in particular for Jay): 1. This patch will make reporter use 2 new prefs (not included in patch: pref("extensions.reporter.privacyURL", "http://reporter-test.mozilla.org/privacy/?plain=true"); pref("extensions.reporter.serviceURL", "http://reporter-test.mozilla.org/service/"); (will change to reporter.mozilla.org when ready) These are *not* included in the patch (I'm not sure where they go so that they are only included when building with reporter). I'm hoping you can take care of that. 2. reporter should still be off until for now (you can checkin with it disabled). I've got a few kinks I'd rather workout before people start hitting the dev instance. 3. Thanks
Comment 11•19 years ago
|
||
This patch will break Firefox localizablity. We should use extensions/reporter/locales/chrome/en-US for locale files probably.
Comment 12•19 years ago
|
||
...and speaking of localization, the patch turned tbox orange because you forgot to define RPT_SHORT in browser/locales/en-US/installer/installer.inc somewhere. Add it somewhere in there within the rest of the defines and that'll fix at least the current bustage (probably all of it, actually, but I didn't actually test a fix).
OS: Windows XP → All
Hardware: PC → All
Reporter | ||
Comment 13•19 years ago
|
||
Just checked in changes to fix the bustage. I'll work with Robert to get his prefs changes in tomorrow.
Reporter | ||
Comment 14•19 years ago
|
||
As it is now Reporter is not enabled by default...you need to specify the extension in mozconfig. Once we're ready to enable it by default, I can edit the configure.in to make it a default extension.
Assignee | ||
Comment 15•19 years ago
|
||
This is apparantly enabled (it's appearing in builds): http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/ this needs to be disabled until we get a few other things done first (I have 2 patches that really need to land first).
Reporter | ||
Comment 16•19 years ago
|
||
This should disable the reporter extension until we're ready for prime time. I'll work with Robert to get the other two checkins in before enabling it again.
Reporter | ||
Comment 17•19 years ago
|
||
r=bryner, sr=ben ...just checked in the changes. We shouldn't see Reporter during the custom install and it should not be installed by default. Please let me know if it doesn't work...I'm still new to this, so learning as I go.
Assignee | ||
Comment 18•19 years ago
|
||
Thanks Jay! I think were going to freeze this for a second until we decide how localization will be dealt with (officially). At that point we'll pick up.
Comment 19•19 years ago
|
||
source l10n patch for reporter tool. It contains fallback to en-US
Attachment #179880 -
Flags: review?(benjamin)
Attachment #179880 -
Flags: approval1.8b2?
Comment 20•19 years ago
|
||
Comment on attachment 179880 [details] [diff] [review] source l10n for reporter a=asa for landing as soon as this has review.
Attachment #179880 -
Flags: approval1.8b2? → approval1.8b2+
Updated•19 years ago
|
Attachment #179880 -
Flags: review?(benjamin) → review+
Comment 21•19 years ago
|
||
I recommend doing this for the URLs, to make for easier testing: All URLs in the client should be accessed through prefs, not via stringbungle strings directly. For l10n reasons you need to do the following: add a pref to your default prefs file in this format: pref("foo.bar", "chrome://reporter/locale/blah.properties"); and in blah.properties: foo.bar=http://reporter.mozilla.org/?blah=%S or whatever you have. Then in the chrome code you want to do: var str = prefBranch.getComplexValue("foo.bar", Components.interfaces.nsIPrefLocalizedString).data; Now, QA can change the values of these prefs easily to test servers via about:config, we can easily deploy update patches that change them if necessary, etc.
Comment 22•19 years ago
|
||
Comment on attachment 179880 [details] [diff] [review] source l10n for reporter checked in
Assignee | ||
Comment 23•19 years ago
|
||
Thanks for that patch. If anyone wants to tackle Ben's suggestion (comment 21), feel free to do so (and accept my thanks). I don't see myself getting to it for a few days due to school. I've got a few other little patches lurking as well.
Reporter | ||
Comment 24•19 years ago
|
||
Robert: I can take a hack at it. Are there any other changes you don't have checked in yet for reporter? If you want to email me your patches, I can make the changes to my tree and submit one huge patch for review when/if I finish before you get time to do it.
Assignee | ||
Comment 25•19 years ago
|
||
Jay: if you can take care of that one: great. I guess we still need a pref or two to point to the correct .properties file as well. I've got a few little things going... but don't worry about them. I'm merging the "generate unique ID" screen into the privacy policy and a few little things. My #1 concern at the moment is localization. So if that can be taken care of, I think we are in decent shape. I'm going to work with justdave on the last server side stuff (production instance). Once that's setup and tested I'll patch so we point to production not dev. So that's the game plan at this moment: 1. Localization 2. Quirks (in the works) 3. Production I've got 2 going, and 3 is in que. If someone can wrap up 1, were covered.
Comment 26•19 years ago
|
||
Didn't we already fix l10n?
Assignee | ||
Comment 27•19 years ago
|
||
This is what I originally had in mind.
Comment 28•19 years ago
|
||
Removing fallback bits because it doesn't work. L10N teams are forced to localize those bits to get their builds.
Attachment #180366 -
Flags: review?(benjamin)
Comment 29•19 years ago
|
||
Comment on attachment 180366 [details] [diff] [review] remove fallback bits Add a newline to the end, please.
Attachment #180366 -
Flags: review?(benjamin) → review+
Comment 30•19 years ago
|
||
Comment on attachment 180366 [details] [diff] [review] remove fallback bits r=bsmedberg;a=asa Checking in extensions/reporter/locales/jar.mn; /cvsroot/mozilla/extensions/reporter/locales/jar.mn,v <-- jar.mn new revision: 1.2; previous revision: 1.1 done
Comment 31•19 years ago
|
||
What's left to do here? Reporter is a 1.8b2 requirement (I hope) and we're running short on time for the release?
Updated•19 years ago
|
Flags: blocking1.8b2+
Assignee | ||
Comment 32•19 years ago
|
||
What's left is figuring out this pref thing (see comment 21). Attachment 180002 [details] [diff] was my attempt... though not sure how great that is. I'm swamped for at least the next week or two, and can't guarantee any time to work on this (time may of course appear magically). Jay sent an email on 4/7 that he had something going as well... perhaps he can take care of this last essential reporter side patch. I'll work with justdave on getting a production instance setup. Then we'll start pointing builds over to that. (this timing isn't ideal for me, so pardon my delays and times of disappearance).
Comment 33•19 years ago
|
||
Have we communicated to the L10n teams that this needs to be localized? Do we know what the status is there - how many of them have localized Reporter?
Comment 34•19 years ago
|
||
(In reply to comment #33) > Have we communicated to the L10n teams that this needs to be localized? Yes. > Do we > know what the status is there - how many of them have localized Reporter? Currently we have 13 locales in CVS trunk, and 9 of them have reporter localized.
Comment 35•19 years ago
|
||
Yes, this has been communicated to npm.l10n, and since it's part of the build system, once bug 287262 is resolved the builds will show red/green/orange whether it has been localized (along with everything else).
Assignee | ||
Comment 36•19 years ago
|
||
We also need a finalized list of problem types. *this effects localization* I need to personally add them as they need to be setup in the server as well. At that time I'm going to clear the reporter-test db so we can do some final testing on a clean slate.
Comment 37•19 years ago
|
||
The finalized list of problem types is this: Browser not supported Can't log in Plugin not shown Other content missing Behavior wrong Appearance wrong Other problem
Assignee | ||
Comment 38•19 years ago
|
||
Requesting approval to land this
Attachment #181341 -
Flags: approval-l10n?
Attachment #181341 -
Flags: approval-aviary1.1a?
Comment 39•19 years ago
|
||
Comment on attachment 181341 [details] [diff] [review] Add new Types a=asa
Attachment #181341 -
Flags: approval-l10n?
Attachment #181341 -
Flags: approval-l10n+
Attachment #181341 -
Flags: approval-aviary1.1a?
Attachment #181341 -
Flags: approval-aviary1.1a+
Assignee | ||
Comment 40•19 years ago
|
||
Checking in extensions/reporter/locales/en-US/chrome/reportWizard.dtd; /cvsroot/mozilla/extensions/reporter/locales/en-US/chrome/reportWizard.dtd,v <-- reportWizard.dtd new revision: 1.2; previous revision: 1.1 done Checking in extensions/reporter/resources/content/reporter/reportWizard.js; /cvsroot/mozilla/extensions/reporter/resources/content/reporter/reportWizard.js,v <-- reportWizard.js new revision: 1.5; previous revision: 1.4 done Checking in extensions/reporter/resources/content/reporter/reportWizard.xul; /cvsroot/mozilla/extensions/reporter/resources/content/reporter/reportWizard.xul,v <-- reportWizard.xul new revision: 1.3; previous revision: 1.2 done
Reporter | ||
Comment 41•19 years ago
|
||
I've tried to get the prefs stuff working, but have had no luck. Talkback related issues are keeping me busy these days, so if we want to get this thing going, someone else needs to step up and finish this off. I'll attach the patch I have (not working), so feel free to work off that. Once we have the extension tested and working, I'll flip the switch so that it is in the nightly builds.
Reporter | ||
Comment 42•19 years ago
|
||
Reporter | ||
Updated•19 years ago
|
Assignee: jay → robert
Assignee | ||
Comment 43•19 years ago
|
||
Use the following prefs for url's to reporter server. pref("extensions.reporter.privacyURL", "chrome://reporter/locale/reportWizard.properties"); pref("extensions.reporter.serviceURL", "chrome://reporter/locale/reportWizard.properties");
Attachment #179239 -
Attachment is obsolete: true
Attachment #180002 -
Attachment is obsolete: true
Attachment #181441 -
Attachment is obsolete: true
Assignee | ||
Comment 44•19 years ago
|
||
Comment on attachment 181710 [details] [diff] [review] Prefs Patch ben, does this look good?
Attachment #181710 -
Flags: review?(bugs)
Comment 45•19 years ago
|
||
OK, what patches here need to be checked in? Also, where are the entries to the default prefs files for the reporter that connect the pref values to the .properties file in the Prefs Patch? I don't see them....
Comment 46•19 years ago
|
||
I really don't think the URLs ought to be in localizable properties files... let's just make it a pref and substitute the locale at runtime. There's a history of localizations making major goofs trying to "translate" URLs.
Comment 47•19 years ago
|
||
this patch fixes reporter for seamonkey
Attachment #181949 -
Flags: review?(benjamin)
Attachment #181949 -
Flags: approval1.8b2?
Comment 48•19 years ago
|
||
*** Bug 291695 has been marked as a duplicate of this bug. ***
Comment 49•19 years ago
|
||
Can we land this now? Is there anything left to do? Time is getting _very_ short for 1.8b2 and I _really_ want to see this make the 1.1 alpha.
Comment 50•19 years ago
|
||
Comment on attachment 181949 [details] [diff] [review] Patch for seamonkey (Checked in) gandalf, feel free to land the seamonkey change as soon as possible.
Attachment #181949 -
Flags: approval1.8b2? → approval1.8b2+
Assignee | ||
Comment 51•19 years ago
|
||
Attachment #181710 -
Attachment is obsolete: true
Attachment #182111 -
Flags: review?
Attachment #182111 -
Flags: approval-l10n?
Attachment #182111 -
Flags: approval-aviary1.1a?
Assignee | ||
Updated•19 years ago
|
Attachment #182111 -
Flags: approval-aviary1.1a? → approval1.8b2?
Updated•19 years ago
|
Attachment #181710 -
Flags: review?(bugs)
Comment 52•19 years ago
|
||
Comment on attachment 182111 [details] [diff] [review] Use Preferences doesn't require further l10n approval, a=asa for 1.8b2. Bens are not available at the moment. Who else can review these two simple patches so we can land this thing?
Attachment #182111 -
Flags: approval1.8b2?
Attachment #182111 -
Flags: approval1.8b2+
Attachment #182111 -
Flags: approval-l10n?
Comment 53•19 years ago
|
||
Comment on attachment 182111 [details] [diff] [review] Use Preferences >+ var prefs = Components.classes["@mozilla.org/preferences-service;1"] >+ .getService(Components.interfaces.nsIPrefService) >+ .getBranch("extensions.reporter."); nit: spacing, but that's common in this file > var soapCall = new SOAPCall(); >- soapCall.transportURI = RMOURI; >+ soapCall.transportURI = prefs.getCharPref("serviceURL"); getCharPref can throw here. What breaks if it does? Unless failure here is handled, try/catch this part of the code please, and the other getCharPref (falling back to the hardcoded values is ok for this case) any call to prefs can fail, there should be a fallback case always. r=me with those fixed.
Attachment #182111 -
Flags: review? → review+
Assignee | ||
Comment 54•19 years ago
|
||
Address mconnor's comments
Attachment #182111 -
Attachment is obsolete: true
Assignee | ||
Comment 55•19 years ago
|
||
Attachment #182121 -
Attachment is obsolete: true
Comment 56•19 years ago
|
||
Comment on attachment 182123 [details] [diff] [review] Use preferences a=asa
Attachment #182123 -
Flags: approval1.8b2+
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Comment 57•19 years ago
|
||
(In reply to comment #50) > (From update of attachment 181949 [details] [diff] [review] [edit]) > gandalf, feel free to land the seamonkey change as soon as possible. Can someone do this on behalf of me? I'm not on my box now. If not, I'll check this in tommorow.
Comment 58•19 years ago
|
||
Comment on attachment 182123 [details] [diff] [review] Use preferences + $(INSTALL) $(srcdir)/resources/content/prefs/reporter.js $(DIST)/bin/defaults/pref PREF_JS_EXPORTS?
Updated•19 years ago
|
Attachment #181949 -
Flags: review?(benjamin) → review+
Comment 59•19 years ago
|
||
Comment on attachment 181949 [details] [diff] [review] Patch for seamonkey (Checked in) Checking in jar.mn; /cvsroot/mozilla/extensions/reporter/locales/jar.mn,v <-- jar.mn new revision: 1.4; previous revision: 1.3 done Checking in generic/chrome/contents.rdf; /cvsroot/mozilla/extensions/reporter/locales/generic/chrome/contents.rdf,v <-- contents.rdf new revision: 1.3; previous revision: 1.2 done
Attachment #181949 -
Attachment description: Patch for seamonkey → Patch for seamonkey (Checked in)
Assignee | ||
Comment 60•19 years ago
|
||
Would someone verify the gandalf's patch (comment 47) for seamonkey worked. This bug should stay open until we enable.
Comment 61•19 years ago
|
||
Can we enable today? Time is getting short on 1.1a.
Comment 62•19 years ago
|
||
(In reply to comment #60) > Would someone verify the gandalf's patch (comment 47) for seamonkey worked. Well, at least there is no parse error displayed in browser windows anymore (I applied the patch manually since CVS mirrors seem to be updating slowly). Anything else to be tested?
Reporter | ||
Comment 63•19 years ago
|
||
Robert: We should probably get it enabled today or tomorrow and see if everything is working. If you need me to check in the changes, let me know. Otherwise you can just revert back the changes I made in https://bugzilla.mozilla.org/attachment.cgi?id=179810
Assignee | ||
Comment 64•19 years ago
|
||
Jay: re-enable, and close this bug. All regressions from here on out go in new bugs under the reporter component. We will point to reporter-test for at least the first builds until we see things are going ok. Please test during htis period. If all goes well I'll point builds from there after to the production database. The move to production will be bug 292696. Thanks to all who have contributed to this bug. Were extremely close. Once were building, and test against reporter-test we'll make the switch.
Comment 65•19 years ago
|
||
Is this supposed to really be on in firefox for real now? It's not (at least on linux). 1. it's not being built on linux according to the ocean build logs. reporter is not in the list of default extensions and is not explicitly listed in about:buildconfig 2. it doesn't seem to be built on windows based on http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1115350920.17763.gz&fulltext=1 3. the unix config.it is missing a line. I'll attach a patch for that.
Comment 66•19 years ago
|
||
Attachment #182750 -
Flags: review?(benjamin)
Assignee | ||
Comment 67•19 years ago
|
||
Andrew: we are *not* enabled as of yet pending a fix for bug 292804. When that is, we will enable it.
Depends on: 292804
Updated•19 years ago
|
Attachment #182750 -
Flags: review?(benjamin) → review+
Updated•19 years ago
|
Attachment #182750 -
Flags: approval-aviary1.1a?
Comment 68•19 years ago
|
||
if we need this config.it fix, a=asa.
Updated•19 years ago
|
Attachment #182750 -
Flags: approval-aviary1.1a? → approval-aviary1.1a+
Comment 69•19 years ago
|
||
Comment on attachment 182750 [details] [diff] [review] fix config.it this is in
Attachment #182750 -
Attachment is obsolete: true
Comment 70•19 years ago
|
||
We're coming up quickly on 1.1a1/1.8b2 and if this doesn't make it, then we'll try again for 1.1a2/1.8b3.
Flags: blocking1.8b2+ → blocking1.8b3+
Reporter | ||
Comment 71•19 years ago
|
||
I've checked in bsmedberg's suggested fix in bug 292804. It worked for him, so let's pray that Reporter is working with tomorrow's build. If not, it's back to the drawing board and we'll have to find some help in getting this thing done.
Comment 72•19 years ago
|
||
> I've checked in bsmedberg's suggested fix in bug 292804. It worked for him, so > let's pray that Reporter is working with tomorrow's build. If not, it's back to > the drawing board and we'll have to find some help in getting this thing done. I'm not sure what you're expecting. If I try to install one of the hourly installer builds from ftp://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/ (http://tinderbox.mozilla.org/Firefox/ tells you what platform the machines are building), and I do not deselect the reporter option, it displays some "can't find reporter.jar" error message. That's because we're still not building reporter by default. So if you want to build Reporter in your own tree, you have to add this line to your mozconfig: ac_add_options --enable-extensions=default,reporter With that, "make installer" in mozilla/browser/installer creates an installer which includes Reporter. If you don't deselect the Reporter option, the build displays the Report item in the Help menu.
Comment 73•19 years ago
|
||
Just in case it isn't clear how to build an extension by default, you have to add it to configure.in these days. For 1.8a2, we need either this patch, or disable Reporter again in the installer. Otherwise, the installer will display an error because it can't find/register Reporter.
Attachment #183724 -
Flags: review?(mconnor)
Updated•19 years ago
|
Flags: blocking1.8b2?
Assignee | ||
Comment 74•19 years ago
|
||
Correct, I believe we also need to re-enable attachment 179810 [details] [diff] [review] Jay: go ahead and turn it on.
Comment 75•19 years ago
|
||
Jay "enabled" Reporter on 2005-05-03 12:49. But unlike before, he only added it to the custom install, but not to the standard install, and also disobeyed the comment about QFA having to be the last in the list: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/installer/windows/config.it&mark=209-210,220-222,225,236-239#209 http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/installer/unix/config.it&mark=215-216,226-228,231,242-245#215
Reporter | ||
Comment 76•19 years ago
|
||
Steffen: You are a lifesaver! I was wondering if I had to add reporter to some other build config file. Thanks for your input and the patch. Once you get a review for your patch, do you want to check it in? Or should I? Also, I asked Asa about enabling reporter for Default installs, and we decided to only include it in Custom installs. Regarding the order of the QFA and RPT, I was going to update that comment to include RPT because both Talkback and Reporter are tools that others might not want to include in their builds (so those two items are fine where they are...at the end of the list). Asa: If this is the last change we need to get this working, let's get it in for 1.8b2.
Comment 77•19 years ago
|
||
For 1.8b2/1.1a1 I'd like to have this be a custom option. For 1.1a2 if everything works out win 1.1a1 testing, then we'll enable by default. Let's get this sucker in! :D
Reporter | ||
Comment 78•19 years ago
|
||
Steffen: I had a question about the configure.in change. When I first hooked up Reporter, I did add "reporter" to the MOZ_EXTENSIONS_ALL list in configure.in. Was that not the right place to put it?
Comment 79•19 years ago
|
||
the MOZ_EXTENSIONS_ALL only gets used if you do --enable-extensions=all. If you don't do that it uses the DEFAULT for whatever product you're building. It's also possible to leave configure alone and do --enable-extensions=default,reporter (on the tinderbox) if you think other people who build from source should not build reporter by default.
Assignee | ||
Comment 80•19 years ago
|
||
(In reply to comment #76) > I was going to update that comment to include RPT because both Talkback and > Reporter are tools that others might not want to include in their builds (so > those two items are fine where they are...at the end of the list). Since that's not code, it's not as urgent, but lets get that done sooner than later.
Assignee | ||
Updated•19 years ago
|
Attachment #183724 -
Flags: review?(mconnor) → review?(chase)
Comment 81•19 years ago
|
||
Feel free to check in the patch once it has review and approval. And we should really try to get that one-liner in before 1.8b2. If we want Reporter to be an option in custom installs, as Asa said, no more changes should be needed - apart from correcting that comment.
Comment 82•19 years ago
|
||
Comment on attachment 183724 [details] [diff] [review] build reporter by default by adding it to configure.in a=asa pending review.
Attachment #183724 -
Flags: approval1.8b2+
Attachment #183724 -
Flags: approval-aviary1.1a1+
Comment 83•19 years ago
|
||
Comment on attachment 183724 [details] [diff] [review] build reporter by default by adding it to configure.in r=chase
Attachment #183724 -
Flags: review?(chase) → review+
Reporter | ||
Comment 84•19 years ago
|
||
Comment on attachment 183724 [details] [diff] [review] build reporter by default by adding it to configure.in Checked in.
Comment 85•19 years ago
|
||
removing blocking flags since this is landed.
Flags: blocking1.8b3+
Flags: blocking1.8b2?
Reporter | ||
Comment 86•19 years ago
|
||
Resolved Fixed. After I made my last checkin, I was able to do a fresh pull and get a build with Reporter working. Please test with 5/17 builds to verify that we are good to go. Thanks to everyone that helped me and Robert out in getting this in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Other Applications → Other Applications Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•