Closed Bug 1304829 Opened 4 years ago Closed 3 years ago

rename RELEASE_BUILD to RELEASE_OR_BETA

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: aryx, Assigned: aryx)

Details

Attachments

(4 files, 1 obsolete file)

It's not obvious that the value of RELEASE_BUILD is true in release and beta builds. There is confusion and uncertainty among a share of the developers, so lets make their lives easier and rename it to RELEASE_OR_BETA.
Attached patch main patch, v1 (obsolete) — Splinter Review
There is also a small patch with changes the check for "release" (mean Release or Beta) in a Spidermonkey test helper function which a spidermonkey peer should review.
Attachment #8793917 - Flags: feedback?(ryanvm)
Comment on attachment 8793917 [details] [diff] [review]
main patch, v1

Sounds OK to me.
Attachment #8793917 - Flags: feedback?(ryanvm) → feedback+
Attached patch main patch, v2Splinter Review
Feel free to redirect.

Try push is: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e97550429fec&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=28066113
Android is busted because it didn't get built with secrets.

Reasoning for change: It's not obvious that RELEASE_BUILD is also true on beta.
Attachment #8793917 - Attachment is obsolete: true
Attachment #8795269 - Flags: review?(ted)
Comment on attachment 8795270 [details] [diff] [review]
spidermonkey test 'release' change, v1

This is a change to code written by you.
Attachment #8795270 - Attachment description: spidermonkey test → spidermonkey test 'release' change, v1
Attachment #8795270 - Flags: review?(arai.unmht)
Comment on attachment 8795270 [details] [diff] [review]
spidermonkey test 'release' change, v1

Review of attachment 8795270 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you :D
Attachment #8795270 - Flags: review?(arai.unmht) → review+
Comment on attachment 8795269 [details] [diff] [review]
main patch, v2

Review of attachment 8795269 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/modules/AppConstants.jsm
@@ +21,5 @@
>  #else
>    false,
>  #endif
>  
> +  RELEASE_OR_BETA:

I feel like you might want a Firefox peer to sign off on this, since this is available to content JS. It's possible we could break extensions if there are any using this.

I note you didn't change the `releaseBuild` property on nsIXULRuntime, so an argument could be made for keeping this as-is (although having the names in AppConstants match the names in the build system is pretty sensible).
Attachment #8795269 - Flags: review?(ted) → review+
Attached patch xpconnect, v1Splinter Review
One-line-change of an at the moment unused variable.
Attachment #8797607 - Flags: review?(gkrizsanits)
Thank you for the review. This two-line patch changes isReleaseBuild to isReleaseOrBeta as suggested in comment 7.
Attachment #8797609 - Flags: review?(ted)
Attachment #8797607 - Flags: review?(gkrizsanits) → review+
Comment on attachment 8795269 [details] [diff] [review]
main patch, v2

Requesting review from Firefox peer as suggested in comment 7.
Attachment #8795269 - Flags: review+ → review?(dtownsend)
Comment on attachment 8795269 [details] [diff] [review]
main patch, v2

Review of attachment 8795269 [details] [diff] [review]:
-----------------------------------------------------------------

I see no extensions using that property. Ted's review is fine for the rest.
Attachment #8795269 - Flags: review?(dtownsend) → review+
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/acc5fe1c96b3
rename RELEASE_BUILD to RELEASE_OR_BETA: main part. r=ted,Mossop
https://hg.mozilla.org/integration/mozilla-inbound/rev/aef916b9ed58
rename Spidermonkey's 'release' to 'release_or_beta'. r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/a41af86326be
Rename test_xrayToJS.xul's isReleaseBuild to isReleaseOrBeta. r=gabor
Comment on attachment 8797609 [details] [diff] [review]
xulruntime's isReleaseBuild

Review of attachment 8797609 [details] [diff] [review]:
-----------------------------------------------------------------

Huh, this isn't used anywhere in the tree? Interesting!
Attachment #8797609 - Flags: review?(ted) → review+
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2451168a497
Rename nsXULAppInfo's isReleaseBuild to isReleaseOrBeta. r=ted
https://hg.mozilla.org/mozilla-central/rev/f2451168a497
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.