980344 - [FTE] Jshint fixes

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

Description

[Fernando Campo (:fcampo)]

Tracking for the needed fixes on jshint for the First Time Experience.

So far this is not a blocker, so let's try get some files out of build/jshint/xfail.list whenever we have the time.

Comment 1

[Fernando Campo (:fcampo)]

Attachment: Link to PR - https://github.com/mozilla-b2g/gaia/pull/16960

First patch, focusing on js, leaving tests for a future one

Comment 2

[Julien Wajsberg [:julienw]]

Comment on attachment 8386833 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/16960

This is great :)

I think you'll want a review from a FTU peer as well, especially if you change some other code like I said :)

Comment 3

[Fernando Campo (:fcampo)]

Comment on attachment 8386833 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/16960

Added few changes after the feedback, so asking for a review to finally merge this.

P.S. Opened bugs for some changes proposed by Julien, I didn't want to add big changes on a jshint fix bug.

Comment 4

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

Comment on attachment 8386833 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/16960

Could you rebase your patch?

Comment 5

[Fernando Campo (:fcampo)]

Comment on attachment 8386833 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/16960

Thought it was rebased, but just updated it anyway. Sorry for that!

Comment 6

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

Comment on attachment 8386833 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/16960

I don't know how this managed to happen but we have linting errors in the ui.js and navigation.js, and I'm saying I don't know how this happened cause we have a precommit hook ;)

Could you fix the linting problems detected and try again to push the branch for travis output?

Comment 7

[Fernando Campo (:fcampo)]

┗(   ⊙ .⊙   )┛ wat?!

ok, fixing

Comment 8

[Fernando Campo (:fcampo)]

Comment on attachment 8386833 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/16960

Travis green.

apparently it was cause by the rebase, that doesn't trigger any hook

Comment 9

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

Comment on attachment 8386833 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/16960

\o/ !!

Now we are talking :)

Thanks a lot Fernando and all the guys involved on this.

Comment 10

[Fernando Campo (:fcampo)]

Cool, first patch merged on master - f4d307c56c2ef1a37d10695ab72000319b293787

Will leave the bug open for further patches (tests, for example)

Thanks Julien and Francisco for your time

Comment 11

[Julien Wajsberg [:julienw]]

(In reply to Fernando Campo (:fcampo) from comment #8)

> apparently it was cause by the rebase, that doesn't trigger any hook

Yeah, there are lots of ways to abuse the hook :)
I'll let you find the other ;) Bottom line is: this is just a tool but it's not perfect.

Comment 12

[Kevin Grandon :kgrandon]

The rest of the FTU jshint issues were resolved in bug 1156607.