Fix name/email address parsing for commits.

Fix name/email address parsing for commits.



https://code.launchpad.net/~bac/juju-gui/bug-1246685/+merge/193450

(do not edit description out of merge proposal)

[bac, 2013-10-31 17:19:38]

Please take a look.

[bac, 2013-10-31 17:26:28]

Author comments

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js
File app/models/bundle.js (left):

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js#oldcode67
app/models/bundle.js:67: /**
Took Jeff's advice and moved back into the class for easy testing access.

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js
File app/models/bundle.js (right):

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js#newcode69
app/models/bundle.js:69: parseNameEmail: function(author) {
Actually we only care about the name as the currently don't display the email
address.  However to match the data collected for charms I chose to go ahead and
squirrel it away.

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js#newcode81
app/models/bundle.js:81: @method extractRecentCommits
I see I didn't document the changes arg.  Will do so.

https://codereview.appspot.com/20170044/diff/1/test/data/browserbundle.json
File test/data/browserbundle.json (right):

https://codereview.appspot.com/20170044/diff/1/test/data/browserbundle.json#n...
test/data/browserbundle.json:9: "Brad Crittenden <bac@example.com>"
Not using a deliverable email address is preferred.

https://codereview.appspot.com/20170044/diff/1/test/data/browserbundle.json#n...
test/data/browserbundle.json:18: "Jorge O. O'Castro <jorge@example.com>",
Took the liberty of making Jorge Irish to catch more special characters.

[rharding, 2013-10-31 17:41:09]

code looks ok, comments below. Will QA now.

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js
File app/models/bundle.js (right):

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js#newcode75
app/models/bundle.js:75: },
space between functions

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js#newcode88
app/models/bundle.js:88: Y.Array.each(changes, function(change) {
if this is an Array, can we just use changes.forEach instead and not need the
YUI module for it?

https://codereview.appspot.com/20170044/diff/1/test/data/browserbundle.json
File test/data/browserbundle.json (right):

https://codereview.appspot.com/20170044/diff/1/test/data/browserbundle.json#n...
test/data/browserbundle.json:18: "Jorge O. O'Castro <jorge@example.com>",
On 2013/10/31 17:26:28, bac wrote:
> Took the liberty of making Jorge Irish to catch more special characters.

well played sir

https://codereview.appspot.com/20170044/diff/1/test/test_model_bundle.js
File test/test_model_bundle.js (right):

https://codereview.appspot.com/20170044/diff/1/test/test_model_bundle.js#newc...
test/test_model_bundle.js:204: var parts = instance.parseNameEmail(
what do you think of naming this more to match the data of
parseAuthorInformation or something?

[rharding, 2013-10-31 17:43:26]

LGTM QA'ok and works with Jorge's bundle now. Thanks for the quick fix!

[bac, 2013-10-31 19:21:03]

*** Submitted:

Fix name/email address parsing for commits.

R=rharding
CC=
https://codereview.appspot.com/20170044

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js
File app/models/bundle.js (right):

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js#newcode75
app/models/bundle.js:75: },
On 2013/10/31 17:41:09, rharding wrote:
> space between functions

I don't know what you mean here.

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js#newcode81
app/models/bundle.js:81: @method extractRecentCommits
On 2013/10/31 17:26:28, bac wrote:
> I see I didn't document the changes arg.  Will do so.

Done.

https://codereview.appspot.com/20170044/diff/1/app/models/bundle.js#newcode88
app/models/bundle.js:88: Y.Array.each(changes, function(change) {
On 2013/10/31 17:41:09, rharding wrote:
> if this is an Array, can we just use changes.forEach instead and not need the
> YUI module for it?

Done.

https://codereview.appspot.com/20170044/diff/1/test/test_model_bundle.js
File test/test_model_bundle.js (right):

https://codereview.appspot.com/20170044/diff/1/test/test_model_bundle.js#newc...
test/test_model_bundle.js:204: var parts = instance.parseNameEmail(
It seems longer and harder to read and not as obvious.  I guess I'm -3.  :)