Closed Bug 1077771 Opened 7 years ago Closed 7 years ago

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

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S7 (24Oct)

People

(Reporter: ShellHacker, Unassigned)

References

Details

Attachments

(1 file)

Errors in the Li test.
Attachment #8503716 - Flags: review?(kgrandon)
Attachment #8503716 - Flags: review?(fabrice)
Comment on attachment 8503716 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/25056

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]
https://github.com/mozilla-b2g/gaia/pull/25056

Updated it and put in line comments.
Attachment #8503716 - Flags: feedback- → feedback?(kgrandon)
Comment on attachment 8503716 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/25056

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: https://github.com/mozilla-b2g/gaia/blob/master/build/jshint/xfail.list

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]
https://github.com/mozilla-b2g/gaia/pull/25056

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]
https://github.com/mozilla-b2g/gaia/pull/25056

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]
https://github.com/mozilla-b2g/gaia/pull/25056

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)
https://github.com/mozilla-b2g/gaia/commit/049d25ec5998a8829aab8e4d6c8d06304d3ebdc0
Status: NEW → RESOLVED
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.