952449 - [Messages] Fix the un-fixed issues reported in the review of Bug #907740

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Julien Wajsberg [:julienw]]

+++ This bug was initially created as a clone of Bug #907740 +++

APP=sms make hint:

$ APP=sms make hint
npm install --registry http://registry.npmjs.org
touch node_modules/.bin/mozilla-download node_modules/.bin/jshint
./node_modules/.bin/jshint  apps/sms 
apps/sms/test/unit/contacts_test.js: line 4, col 1, 'mocha' is not defined.
apps/sms/test/unit/contacts_test.js: line 165, col 31, 'MockFbReaderUtilsObj' is not defined.
apps/sms/test/unit/contacts_test.js: line 478, col 7, 'fb' is not defined.

apps/sms/js/startup.js: line 7, col 63, 'fb' is defined but never used.

apps/sms/js/desktop-only/contacts.js: line 11, col 82, Line is too long.

apps/sms/js/contacts.js: line 243, col 9, 'fb' is not defined.

apps/sms/js/activity_handler.js: line 441, col 85, Line is too long.

apps/sms/js/utils.js: line 19, col 36, Did you mean to return a conditional instead of an assignment?

apps/sms/js/waiting_screen.js: line 11, col 27, Did you mean to return a conditional instead of an assignment?
apps/sms/js/waiting_screen.js: line 15, col 33, Did you mean to return a conditional instead of an assignment?

apps/sms/js/action_menu.js: line 74, col 89, Line is too long.

11 errors
make: *** [hint] Erreur 2

The "not defined" issues needs to be fixed so that we're back at 6 errors.

Comment 1

[Jose Manuel Cantera]

Attachment: 14894.html

my Christmas gift :)

Comment 2

[Julien Wajsberg [:julienw]]

Comment on attachment 8350577 [details]
14894.html

Hey José,

thanks a lot, I really appreciate it (and I mean it!)

Comment 3

[Jose Manuel Cantera]

merged in master:

https://github.com/mozilla-b2g/gaia/commit/b49b1893e1261548f25153c06ce20696cf176a24

Comment 4

[Jose Manuel Cantera]

Comment on attachment 8350577 [details]
14894.html

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): hint problems
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky): No risk
[String changes made]:

Asking also for approval in order to keep hint consistency as requested by SMS app owners

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Conditioning this approval to bug 907740

Comment 6

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8350577 [details]
14894.html

Leaving decision to release engineers

Comment 7

[Julien Wajsberg [:julienw]]

This changes only comments, I'd say, just uplift it without approval to prevent future conflicts.

Comment 8

[bhavana bajaj [:bajaj]]

(In reply to Francisco Jordano [:arcturus] from comment #6)
> Comment on attachment 8350577 [details]
> 14894.html
> 
> Leaving decision to release engineers
Given 907740 was never approved I am assuming we don't need this.  Also at this point we are limiting approvals to only 1.3 blocker bugs. Hence unless there is a direct user impact here lets resolve this in master