Closed Bug 1077771 Opened 7 years ago Closed 7 years ago

Fix jshint error in apps/bluetooth/js/deviceList.js


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

Not set


(Not tracked)

2.1 S7 (24Oct)


(Reporter: ShellHacker, Unassigned)




(1 file)

Errors in the Li test.
Attachment #8503716 - Flags: review?(kgrandon)
Attachment #8503716 - Flags: review?(fabrice)
Comment on attachment 8503716 [details] [review]

I am not sure if we have an official peer of the bluetooth code, but I think this needs more work before a review pass. I've left a comment on github, we should rarely use the exceptions where possible, and they should be documented. Please re-flag me for feedback if you'd like once this is addressed.
Attachment #8503716 - Flags: review?(kgrandon) → feedback-
Comment on attachment 8503716 [details] [review]

Updated it and put in line comments.
Attachment #8503716 - Flags: feedback- → feedback?(kgrandon)
Comment on attachment 8503716 [details] [review]

Thank you for the patch. Unfortunately for our CI to process this file, it needs to be removed from xfail.list, otherwise this doesn't have much benefit. You can find xfail.list here:

I've also added some additional comments on github. We should not be using these jshint flags. We should address the problem so we don't need these special flags.

Please address and re-flag me. Thanks!
Attachment #8503716 - Flags: feedback?(kgrandon) → feedback-
Comment on attachment 8503716 [details] [review]

Clearing the review flag on Fabrice for now as we need these issues addressed before the review pass. Thanks!
Attachment #8503716 - Flags: review?(fabrice)
Pushed the changes.
Flags: needinfo?(kgrandon)
Comment on attachment 8503716 [details] [review]

Feedback+. Forwarding the review for Ian to do though. Thank you for your contribution!
Attachment #8503716 - Flags: review?(iliu)
Attachment #8503716 - Flags: feedback-
Attachment #8503716 - Flags: feedback+
Flags: needinfo?(kgrandon)
Comment on attachment 8503716 [details] [review]

The patch is working fine with my manual test. And there is no jshint error after applied it. But, have to make sure all tests passed on try-server. Thanks for Sudheesh's contribution here. :) r+ with me
Attachment #8503716 - Flags: review?(iliu) → review+
Hi, can't merge this pull request, seems there are merge conflicts. Could you guys take a look, thanks!
Keywords: checkin-needed
Hi Carsten,

Rebased, cleared merge conflict. Good to merge
Flags: needinfo?(cbook)
Closed: 7 years ago
Flags: needinfo?(cbook)
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S7 (24Oct)
You need to log in before you can comment on or make changes to this bug.