superreview requested: [Bug 755069] Update BreakpadWrapper for forthcoming Scorro "API" changes : [Attachment 623844] Update to new channel and version string requirements, v1.0

[bugzill...@mozilla.org]

Smokey Ardisson (not following bugs - do not email) <alqa...@ardisson.org> has
asked Stuart Morgan <stuart.morg...@gmail.com> for superreview:
Bug 755069: Update BreakpadWrapper for forthcoming Scorro "API" changes
https://bugzilla.mozilla.org/show_bug.cgi?id=755069

Attachment 623844: Update to new channel and version string requirements, v1.0
https://bugzilla.mozilla.org/attachment.cgi?id=623844&action=edit


------- Additional Comments from Smokey Ardisson (not following bugs - do not
email) <alqa...@ardisson.org>
The Socorro team has notified us that Socorro is going to discontinue support
for anything that doesn't conform to the current Firefox release/version scheme
shortly. However, with a few small changes on our end and a couple of small
changes on their end to allow them to not completely abandon their open-source
users, we can continue submitting crashes and getting reports.

Basically, the requirements are:
* Must send a valid Firefox release channel
* Version numbers must be no more than three parts (no more Firefox 2.0.0.9!)
* Version numbers must be X.Y.Z or X.YaZ or X.YbZ

Which translates into the following action steps for us:
* Remove "pre" from the version number we send to Socorro for X.Y.Zpre,
X.YaZpre, and X.YbZpre versions
* All "pre" versions are assigned to the "nightly" channel
* Any "a" milestone versions are assigned to the "aurora" channel
* Any "b" milestone versions are assigned to the "beta" channel
* Any release versions are assigned to the "release" channel

The following patch does just that. Herewith sample crashes, with the actual
CFBundleShortVersionString in parens in the report's comments, to show
version-munging and channel-assigning in action:

2.1.3pre: bp-d302fa1d-40e6-4594-a8e1-bf7e82120514
2.1.3: bp-7883bc52-ebae-4c1a-b231-7fa432120514
2.2a1pre: bp-4cca9a3a-1099-49e2-bd3b-f02772120514
2.2a1: bp-4cc024eb-8cc1-4aaa-8097-ca36c2120514
2.2b2pre: bp-650e4d21-2ad9-4c1e-9528-eac792120514
2.2b2: bp-8b5ad7c3-ce69-4c93-8bfc-eb95b2120514
2.2: bp-693429a9-67d8-4b84-b4f0-abacc2120514
2.2.1pre: bp-1b785b17-e704-44c2-8e40-bc2d62120514

As the question-comment in the code indicates, I don't know if we need a better
check for the real release versions; we currently should never have anything
outside the parameters already checked for, so I don't know if
future-proofing/error-proofing is necessary.

If it is necessary, is the best way to create an NSCharacterSet of digits and
period, invert it, and then make sure that
[cmVersion rangeOfCharacterFromSet:invertedDecimalAndPeriodSet].location ==
NSNotFound?

[bugzill...@mozilla.org]

Stuart Morgan <stuart.morg...@gmail.com> has granted Smokey Ardisson
(not following bugs - do not email) <alqa...@ardisson.org>'s request for
superreview:
Bug 755069: Update BreakpadWrapper for forthcoming Socorro "API" changes

https://bugzilla.mozilla.org/show_bug.cgi?id=755069

Attachment 623844: Update to new channel and version string requirements, v1.0
https://bugzilla.mozilla.org/attachment.cgi?id=623844&action=edit

------- Additional Comments from Stuart Morgan
<stuart.morg...@gmail.com>
>+ NSString* breakpadVersion = nil;

Let's do breakpadVersion = cmVersion (instead of nil) here, and then just use
breakpadVersion unconditionally in the SetKeyValue later.

>+ // XXXsa should we have a better check here to make sure this is really
a
>+ // release?

No, I think the structure you have is good.


sr=smorgan