…Because we have a ton of existing code that
Does things like `const $user_name = $('#user_name’)`
And it doesn’t make anything “better” to force people to
go around renaming all those. And, although with BEM / React
you probably wouldn’t write code like that in new stuff,
If the DOM ID really is #user_name, for greppability it arguably
is better to keep the js variable and the dom id the same.
(or at least it is not objectively worse than having one with
underscores and one in camelCase).
Change-Id: I042deb3a2bd612eaf74addb141015d288a7330fe
Reviewed-on: https://gerrit.instructure.com/112176
Tested-by: Jenkins
Reviewed-by: brian kirkby <bkirkby@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Closes: CNVS-36236
We now treat code in public/javascripts
exactly the same as app/jsx. Since we pass
it all to babel we can lint with the same rules
as we do app/jsx and you can use all the modern
syntax/features/babel goodness there. There is literally
no excuse to write old crapy javascript anymore.
Now we can start adding some commits that eslint -fix all the existing
stuff in public javscripts.
Test plan:
* if you run eslint public/javascripts/instructure.js it should
give you a bunch of warnings about converting things from var
to let/const and other stuff like that.
Change-Id: I5d400b958ae33f3f13f7f6a9c3505a4f7b5843db
Reviewed-on: https://gerrit.instructure.com/112146
Tested-by: Jenkins
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Test Plan:
- Breaking those rules will be okay or in the case of
named stuff it fails :)
Change-Id: I5d41ec8d12aee5a59b6f35da0a86ed1571950998
Reviewed-on: https://gerrit.instructure.com/100877
Tested-by: Jenkins
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Product-Review: Clay Diffrient <cdiffrient@instructure.com>
QA-Review: Clay Diffrient <cdiffrient@instructure.com>
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
closes: CNVS-34124
These are it’s default rules:
"rules": {
"promise/always-return": "error",
"promise/no-return-wrap": "error",
"promise/param-names": "error",
"promise/catch-or-return": "error",
"promise/no-native": "off",
"promise/no-nesting": "warn",
"promise/no-promise-in-callback": "warn",
"promise/no-callback-in-promise": "warn",
"promise/avoid-new": "warn"
}
the only one I changed was set avoid-new to ignore
because I have noticed that there was a few times
where I really did need/want to make my own promise.
I am open to debate on what others think though
Test Plan:
* the eslint errors on your code that uses promises
should have the warnings you expect
Change-Id: I46e5f2d1d2ded2dba1e5aeb2ebcf303bcd4c5981
Reviewed-on: https://gerrit.instructure.com/98201
Tested-by: Jenkins
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Test Plan:
- Using ENV should not show as a problem anywhere
Change-Id: I4b37a95606755e21716df0dfb61f26a85e295fe9
Reviewed-on: https://gerrit.instructure.com/98198
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Tested-by: Jenkins
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
This makes it so eslint handles situations where we don't have
ES2015+ available, while still allowing us to have our own common
rules.
Change-Id: I86b49a32433970afbcc48ef0f6fcdd7301795b63
Reviewed-on: https://gerrit.instructure.com/97872
Tested-by: Jenkins
Reviewed-by: Dan Minkevitch <dan@instructure.com>
Product-Review: Clay Diffrient <cdiffrient@instructure.com>
QA-Review: Clay Diffrient <cdiffrient@instructure.com>
Ryan Shaw