Closed Bug 202315 Opened 21 years ago Closed 15 years ago

remove debug UI from final release branches

Categories

(SeaMonkey :: Build Config, defect, P1)

Product:
SeaMonkey
Component:
Build Config
Platform:
x86
Windows 2000
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
seamonkey2.0b1

People

(Reporter: leaf, Assigned: kairo)

References

Details

(6 keywords)

Attachments

(2 files, 2 obsolete files)

remove debug UI for composer/browser, buildid from titlebar, debug prefs
5.02 KB, patch
chofmann
: approval1.7+
kairo
: approval-seamonkey1.0+
Details | Diff | Splinter Review
only build debugQA in alpha and nightly builds, v1.1
574 bytes, patch
kairo
: review+
standard8
: approval-seamonkey2.0b1+
Details | Diff | Splinter Review 
Open perma-bug to track removing UI that deals with debug functionality in final
release builds.
Status: NEW → ASSIGNED
lack of final branch milestone targets is lame.
Target Milestone: --- → mozilla1.5alpha
Priority: -- → P1
Target Milestone: mozilla1.5alpha → mozilla1.6alpha
Target Milestone: mozilla1.6alpha → mozilla1.6beta
*poke*
Flags: blocking1.6+
Target Milestone: mozilla1.6beta → mozilla1.6final
This seems to have been done for 1.6 branch now. not resolved because it's
supposed to be a "perma-bug", but I guess fixed1.6 is appropriate.
Keywords: fixed1.6
I see the same as Comment 3.  

Also nice to see W32 builds back for /latest-1.6
this has been fixed now so taking off the 1.6+ and moving to 1.7+
Flags: blocking1.6+ → blocking1.7a+
Flags: blocking1.7a+ → blocking1.7a-
Should not Debug and QA menus be deleted in RCs? It is not the case in 1.7 RC1.
well RC1 isn't a true candidate for release (there are plans for another 2
release candidates), so it doesn't matter too much.  However, the debug UI
should be removed from the 1.7 branch before the last release candidate.
Flags: blocking1.7?
I understand. In this case, bug 241306 depends (more or less) on this one, right?

Not really, no. The release note link and versions are handled separately from
this UI stuff. They both need to happen, but there's no dependency - either can
be fixed without the other.
Flags: blocking1.7? → blocking1.7+
Comment on attachment 147047 [details] [diff] [review]
remove debug UI for composer/browser, buildid from titlebar, debug prefs

not sure who i should ask for review, but blizzard's sr should be sufficient.
Attachment #147047 - Flags: superreview?(blizzard)
Attachment #147047 - Flags: approval1.7?
Comment on attachment 147047 [details] [diff] [review]
remove debug UI for composer/browser, buildid from titlebar, debug prefs

a=chofmann for 1.7, just do it ( since it has been done so many times before
;-)
Attachment #147047 - Flags: approval1.7? → approval1.7+
changes committed to 1.7 branch; i will run this morning's builds when they get
done and make sure no additional debug items have cropped up.
Keywords: fixed1.7
Attachment #147047 - Flags: superreview?(blizzard)
status need to be updated ?
No - it's already marked fixed for 1.7. It will need to be done again on the 1.8
branch.
So the blocking1.7+ flag can go down, cannot it?
If someone is offended by it being there, I guess it could.  Usually it's left
as a record of the fact that it was a blocker - the fixed1.7 keyword indicates
that it's been fixed for 1.7.
I believe the whole point of the blocking flags is to find the bugs that still
block a release.

Otherwise, why at present are there only 5 blocking1.7+ bugs when 10 days ago
there were 17 (or something like that)?  
There aren't 5 blockers - there are none now.

You're looking at the resolved status of the bug, which relates to whether
the bug is fixed on the trunk or not.  The status of the bug on the branch
is indicated by the "fixed1.7" keyword. The correct queries to use are linked from
http://www.mozilla.org/roadmap/release-status.html

This discussion doesn't belong in this bug anyway - you're welcome to email me
if you want to reply.
I never said there are 5 blockers. I said the drivers actually _do_ remove the
flag after fixing a blocker. The proof is that there are now only 5 blocking1.7+
bugs when recently there were about 20. It seems you read my comment in a very
cursory manner, indeed.

No. I actually do not need your reply. Thanks for the offer, anyway. And sorry
for the spam the above mentioned misunderstanding has created.
Verified in Mozilla 1.7 branch. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7) Gecko/20040617. Debug UI removed.
Keywords: fixed1.7 → verified1.7
Out of curiosity, instead of doing this for every release cycle, why not just
put the debug UI inside #ifndef MOZ_OFFICIAL or whatnot?
Assignee: leaf → cmp
Status: ASSIGNED → NEW
Product: Browser → Seamonkey
Fwiw, attachment #147047 [details] [diff] [review] still applies.
Flags: blocking-seamonkey1.0?
We should look into that after Beta, maybe we can even wait for a point when the 1.8.1 branch was already cut (dunno when that will be)
Flags: blocking-seamonkey1.0? → blocking-seamonkey1.0+
Comment on attachment 147047 [details] [diff] [review]
remove debug UI for composer/browser, buildid from titlebar, debug prefs

first a=me for 1.0, need one more

(tested and it seems to work)
Whiteboard: [patch still works for seamonkey]
Attachment #147047 - Flags: approval-seamonkey1.0+
Checked in on 1.8.0
Whiteboard: [patch still works for seamonkey] → fixed-seamonkey1.0
Keywords: fixed1.8.0.1
Mass reassign of open bugs for chase@mozilla.org to build@mozilla-org.bugs.
Assignee: chase → build
Keywords: fixed-seamonkey1.0
Whiteboard: fixed-seamonkey1.0
Chris - sending this bug your way - just to confirm if we need this on the trunk /1.8 branches or is this issue closed?
Assignee: build → cst
(In reply to comment #28)
> Chris - sending this bug your way - just to confirm if we need this on the
> trunk /1.8 branches or is this issue closed?
> 

As I understand it, we will need this for every release on every branch except 1.8.0.x-based ones.  I was under the impression this is a bug that won't ever get resolved.
Flags: blocking-seamonkey1.1?
Flags: blocking-seamonkey1.1? → blocking-seamonkey1.1+
bienvenu, this is going to affect Thunderbird again.  Is that ok with you?
CTho - how does it affect TB? The editor.xul change?
(In reply to comment #31)
> CTho - how does it affect TB? The editor.xul change?
> 

I guess so.  Last time I landed it, someone said I affected TB.  I don't remember how exactly.
Fixed for seamonkey 1.1 on mozilla1.8 branch.
Keywords: fixed-seamonkey1.1, fixed1.8.1
Depends on: 381343
standard8, you're bitrotting these patches, aren't you?
CTho, his work in bug 381343 is surely bitrotting this as it's trying to finally fix this in a way that we only need to disable an extension and achieve the same :)
(In reply to comment #34)
> standard8, you're bitrotting these patches, aren't you?
> 
Yep, like Robert said, bug 381343 is moving the debug/QA UI and functions to an extension so that all we'll have to do is disable building of an extension for releases, if not automatically.

It'll also mean that our release builds are slightly less bloated by debug functionality (though that is probably small in comparison with the rest of the build - but still worth doing it). The other advantage is that we could publish the new extension on amo so that testers can get debug & qa functions for release builds.

If we're coming to a release and we need to do some commenting out and I haven't finished the extension, then I'll be happy to supply updated patches, but I'm hoping to have the extension "finished" by then.
I believe SeaMonkey now has all the debug and QA code that used to be disabled for releases (by the patch on this bug) in one extension under suite/debugQA.

It can be excluded from the build by commenting out its entry in suite/Makefile.in.

Do we wish to do that normally, or do we want to make building it depend on version.txt containing "pre"? - so for releases (e.g. change of version to 2.0a1) it would automatically not be built, and hence one less thing for us to do at release time?
Patch based on my comment 37. This will only build the debugQA extension if "pre" is found in the current version.txt file.

The windows installer will already cope with not having the debugQA extension built, so there should be no issues there.

This would mean that a) we can close this bug, b) we can keep building debugQA on branches (or trunk) until very close to the actual release, c) we don't have to do anything to the build apart from change the version number.
Attachment #281099 - Flags: superreview?(neil)
Attachment #281099 - Flags: review?(kairo)
Attachment #281099 - Flags: review?(cst)
Comment on attachment 281099 [details] [diff] [review]
Automatically disable debugQA for non-"pre" builds of suite.

IIRC, we always did ship the debug UI for alpha and beta build as well.

Additionally, it would be really nice to have a simple way for someone doing custom builds to still build the extension, even from release code.
Attachment #281099 - Flags: review?(kairo) → review-
(In reply to comment #39)
> (From update of attachment 281099 [details] [diff] [review])
> IIRC, we always did ship the debug UI for alpha and beta build as well.

IMHO if we're going to ship the debug UI in alpha and beta builds then we should be shipping it finals as well. Reasoning: alpha and beta builds are previews of what will be in the final builds. Its very rare you'd remove a feature between beta and final. If we ship debug UI for alpha and beta, and not final, then I can see some confused users coming.

> Additionally, it would be really nice to have a simple way for someone doing
> custom builds to still build the extension, even from release code.

Agreed, any suggestions on how we do this within the limits of the build system?
I think we shipped alphas and betas with the debug UI as long as we've had those for the suite, and I know for sure we did so for SeaMonkey. I don't remember any complaints about not having it in stable releases.

For enabling it, we could do that with some Makefile variable, probably.
Comment on attachment 281099 [details] [diff] [review]
Automatically disable debugQA for non-"pre" builds of suite.

I'm not sure of how we want to deal with this currently, or of the best fix for it. Maybe we should just leave this bug open and patch it manually each time.
Attachment #281099 - Flags: superreview?(neil)
-> somebody else
Assignee: cst → nobody
QA Contact: granrosebugs → build-config
I came across this again and had an idea for a way to do this without even calling into a subshell, and for alphas and nightlies (i.e. those with "pre" in the version) only.
Here is the patch to do that, I tested it with a couple of different version strings.
Assignee: nobody → kairo
Attachment #281099 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #388091 - Flags: review?(neil)
Comment on attachment 388091 [details] [diff] [review]
only build debugQA in alpha and nightly builds

>+ifneq (,$(filter xxx,$(subst a, xxx ,$(subst pre, xxx ,$(MOZ_APP_VERSION)))))
Does $(filter a pre,$(MOZ_APP_VERSION)) work?
So, this means that debugQA won't be in the upcoming beta, right? Hmm, then we need to fix bug 457548...
(In reply to comment #45)
> (From update of attachment 388091 [details] [diff] [review])
> >+ifneq (,$(filter xxx,$(subst a, xxx ,$(subst pre, xxx ,$(MOZ_APP_VERSION)))))
> Does $(filter a pre,$(MOZ_APP_VERSION)) work?

No, as filter works on space-separated words.

(In reply to comment #46)
> So, this means that debugQA won't be in the upcoming beta, right? Hmm, then we
> need to fix bug 457548...

SeaMonkey always removed the debug UI for betas. And I thought  everything that could have been done so far has been done for that bug? If not, the beta will ship with the bug unless you fix it before Tuesday.
(In reply to comment #47)

> SeaMonkey always removed the debug UI for betas.

SeaMonkey have not "always removed debug UI for betas" according to your comment #41.

> And I thought  everything that
> could have been done so far has been done for that bug? If not, the beta will
> ship with the bug unless you fix it before Tuesday.

I requested blocking-2.0 in October 2008 (no reaction) because I assumed debug UI was not going to be removed until we shipped. If I had known that it was going to be removed for beta I would have fixed the bug for a long time ago :(
(In reply to comment #48)
> SeaMonkey have not "always removed debug UI for betas" according to your
> comment #41.

I think it's right to only ship it in alphas and nightlies, esp. as that means that betas make a testing ground for things like the window title stuff you brought up.

> I requested blocking-2.0 in October 2008 (no reaction) because I assumed debug
> UI was not going to be removed until we shipped. If I had known that it was
> going to be removed for beta I would have fixed the bug for a long time ago :(

And I didn't think of it as I for one thing am not on a Mac and for the other, I thought all the patching in that bug had fixed the issue already.
(In reply to comment #47)
> (In reply to comment #45)
> > (From update of attachment 388091 [details] [diff] [review] [details])
> > >+ifneq (,$(filter xxx,$(subst a, xxx ,$(subst pre, xxx ,$(MOZ_APP_VERSION)))))
> > Does $(filter a pre,$(MOZ_APP_VERSION)) work?
> No, as filter works on space-separated words.
I see I misunderstood, and I reread the manual, and perhaps this works:
($filter %a %pre,$(MOZ_APP_VERSION))
Does this affect the installer at all?
(In reply to comment #50)
> I see I misunderstood, and I reread the manual, and perhaps this works:
> ($filter %a %pre,$(MOZ_APP_VERSION))

It works for the "pre" part, but not for the "a", a version of e.g. 2.0a1 doesn't trigger it.

(In reply to comment #51)
> Does this affect the installer at all?

The installer is made to degrade gracefully if debugQA isn't built, from all I know.
Comment on attachment 388091 [details] [diff] [review]
only build debugQA in alpha and nightly builds

>+# replacing the "a" or "pre" in the version with <sacpe>xxx<space> enables us
OK, so make sucks, but I don't like xxx - maybe <space>debugQA</space>?

P.S. Actually I think $(findstring a,$(MOZ_APP_VERSION:pre=a)) works.
Attachment #388091 - Flags: review?(neil) → review+
Depends on: 462997
OK, Neil's last suggestion works, thanks. Forwarding r+ and requesting approval for 2.0b1.
Attachment #388091 - Attachment is obsolete: true
Attachment #388240 - Flags: review+
Attachment #388240 - Flags: approval-seamonkey2.0b1?
Attachment #388240 - Flags: approval-seamonkey2.0b1? → approval-seamonkey2.0b1+
Pushed as http://hg.mozilla.org/comm-central/rev/57a5d25e6406

This should finally fix this bug once and for all. yay!
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.6final → seamonkey2.0b1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: