Closed
Bug 1206683
Opened 9 years ago
Closed 9 years ago
Enable eslint rules for Loop: No unused variables
Categories
(Hello (Loop) :: Client, defect, P3)
Hello (Loop)
Client
Tracking
(firefox45 fixed)
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: standard8, Assigned: standard8, Mentored)
References
Details
(Whiteboard: [lang=js][tech-debt][good first bug])
Attachments
(1 file)
34.73 KB,
patch
|
Mardak
:
review+
|
Details | Diff | Splinter Review |
Enable some more eslint rules for Loop. Rules to enable in this bug:
* no-unused-vars - disallows unused variables
To enable the rule, remove the line from browser/components/loop/.eslintrc and save it.
Then you can run eslint in the browser/components/loop directory with:
eslint --ext .js --ext .jsm --ext .jsx .
The errors listed in the output are to be fixed.
If there's errors in a .js file that has an associated .jsx file, then fix the jsx file and run the react tools to generate the .js file (https://wiki.mozilla.org/Loop/Development#Developing).
If you need more help setting up eslint, see https://wiki.mozilla.org/Loop/Development#Additional_Requirements
Information about the eslint rules can be found here:
http://eslint.org/docs/rules/
Updated•9 years ago
|
Rank: 35
Priority: -- → P3
Thanks
Can you please guide me how to
Change
c:\Users\moztest\AppData\Roaming
to /c/Users/moztest/AppData/Roaming
on widows platform
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Hussain from comment #3)
> Thanks
> Can you please guide me how to
>
> Change
> c:\Users\moztest\AppData\Roaming
You need to enter the following on the console:
echo $APPDATA
Then in whatever that prints out, you need to change the '\' to '/', add one '/' at the beginning and drop the ':'.
Hi
I have use the
echo $PATH:/c/program\ files/nodejs:/c/Users/hussain/AppData/Roaming/npm >> ~/.profile
But I get error following error when i use npm install -g react-tools@0.12.2 command
bash: npm: command not found
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to Hussain from comment #5)
> Hi
> I have use the
> echo $PATH:/c/program\ files/nodejs:/c/Users/hussain/AppData/Roaming/npm >>
> ~/.profile
>
> But I get error following error when i use npm install -g react-tools@0.12.2
> command
> bash: npm: command not found
Did you restart the shell/command prompt after doing this?
Yes I did restart shell. Is there another way to address this problem
Assignee | ||
Comment 8•9 years ago
|
||
I've just realised that the wiki page is wrong. I've now corrected it. Sorry about that.
What you'll need to do is to edit ~/.profile, and change:
$PATH:/c/program\ files/nodejs:/c/Users/hussain/AppData/Roaming/npm
to:
PATH=$PATH:/c/program\ files/nodejs:/c/Users/hussain/AppData/Roaming/npm
and then restart the command prompt again. That should hopefully fix it.
Thank you for your help
Just an other other are these two action optional?
/path/to/firefox -createProfile myTestProfile
./mach run -P myTestProfile -purgecaches
Also is /path/to/firefox mean firefox installed on machine or firefox folder in mozilla-central
Thanks
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Hussain from comment #9)
> Thank you for your help
> Just an other other are these two action optional?
>
> /path/to/firefox -createProfile myTestProfile
This is used to create a new test profile. You don't have to create a separate one, but it keeps your testing/development separate from your main one.
> ./mach run -P myTestProfile -purgecaches
You can do just
./mach run -purgecaches
to start Firefox with the default profile.
> Also is /path/to/firefox mean firefox installed on machine or firefox
> folder in mozilla-central
It would the the one installed on your machine.
Assignee | ||
Comment 11•9 years ago
|
||
Hussain, are you still willing/able to work on this? Please let us know either way.
Flags: needinfo?(hussain237)
Assignee | ||
Comment 12•9 years ago
|
||
Sorry Hussain, its been a few weeks now and I haven't heard anything. We want to move this bug forward to help the project, so I'm going to take it. If you want to work on other bugs in the future, please do. This one has become a slightly higher priority as we need to be cleaning our code better.
Assignee: hussain237 → standard8
Flags: needinfo?(hussain237)
Assignee | ||
Comment 13•9 years ago
|
||
Attachment #8696594 -
Flags: review?(mdeboer)
Attachment #8696594 -
Flags: review?(edilee)
Attachment #8696594 -
Flags: review?(dmose)
Comment 14•9 years ago
|
||
Comment on attachment 8696594 [details] [diff] [review]
No unused variables.
Review of attachment 8696594 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/extensions/loop/content/shared/js/linkifiedTextView.jsx
@@ -4,5 @@
>
> var loop = loop || {};
> loop.shared = loop.shared || {};
> loop.shared.views = loop.shared.views || {};
> -loop.shared.views.LinkifiedTextView = (function(mozL10n) {
Do we want to get rid of this? Might be nice to consistently expect mozL10n.
::: browser/extensions/loop/content/shared/js/otSdkDriver.js
@@ -991,4 @@
> * which to copy the stream when attaching it to visible video element
> * that the views control directly.
> *
> * @param event {OT.VideoEnabledChangedEvent} from the SDK
@param event comment should be removed
@@ -1011,4 @@
> * Handle the SDK disabling of remote video by dispatching the
> * appropriate event.
> *
> * @param event {OT.VideoEnabledChangedEvent) from the SDK
same @param event
Attachment #8696594 -
Flags: review?(mdeboer)
Attachment #8696594 -
Flags: review?(edilee)
Attachment #8696594 -
Flags: review?(dmose)
Attachment #8696594 -
Flags: review+
Assignee | ||
Comment 15•9 years ago
|
||
(In reply to Ed Lee :Mardak from comment #14)
> > -loop.shared.views.LinkifiedTextView = (function(mozL10n) {
>
> Do we want to get rid of this? Might be nice to consistently expect mozL10n.
Well, we can always add it back later ;-) Though I did look at that component and think it unlikely that we'll ever need l10n in there due to its limited scope.
Comment 17•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•