Kyle, I'm presuming this is referring to bug 591387 comment 21? I'm slighly confused however due to bug 591387 comment 26 and also the fact that there is already: AC_DEFINE_UNQUOTED(MOZ_UA_FIREFOX_VERSION, "$FIREFOX_VERSION" ...so why do we need this defined again as FIREFOX_VERSION? I'm happy to do this cleanup, just unclear at present what of: http://mxr.mozilla.org/mozilla-central/search?string=FIREFOX_VERSION ...needs changing to what. Thanks!
MOZ_UA_FIREFOX_VERSION is something that's semantically different, even if it has the same value (it also has quotes). I'd like to not touch that, and instead AC_DEFINE_UNQUOTED(FIREFOX_VERSION,$FIREFOX_VERSION). Once that's done, any makefile that does something like DEFINES += \ -DFIREFOX_VERSION=$(FIREFOX_VERSION) can have that bit removed. As for bug 591387 comment 26, we can remove the AC_SUBST(MOZ_UA_FIREFOX_VERSION ...) and remove the autoconf.mk.in bit. I would of course be happy to review.
Created attachment 535422 [details] [diff] [review] Patch v1 - Removes |AC_SUBST(MOZ_UA_FIREFOX_VERSION)| in both configure.ins and the corresponding |MOZ_UA_FIREFOX_VERSION = @FIREFOX_VERSION@| in autoconf.mk.in , since MOZ_UA_FIREFOX_VERSION is not used in a Makefile anywhere (http://mxr.mozilla.org/mozilla-central/search?string=MOZ_UA_FIREFOX_VERSION) - Adds |AC_DEFINE_UNQUOTED(FIREFOX_VERSION,$FIREFOX_VERSION)| to /configure.in, and then removes all |-DFIREFOX_VERSION=$(FIREFOX_VERSION)| (http://mxr.mozilla.org/mozilla-central/search?string=DFIREFOX_VERSION) since they are now redundant.
Assignee: khuey → bmo
Status: NEW → ASSIGNED
Attachment #535422 - Flags: review?(khuey)
8 years ago
Attachment #535422 - Flags: review?(khuey) → review+
Version: unspecified → Trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0 Is there any way I can verify this? I looked in the files in the repo: http://hg.mozilla.org/mozilla-central/file/e0acef471ab2 Is that enough to mark this as VERIFIED FIXED? Thanks!
You need to log in before you can comment on or make changes to this bug.