Implement style guide / stick to it
No description provided.
Comments (20)
-
-
-
Account Deleted reporter Beautiful
-
- changed milestone to 1.0 Release
-
Agree on ESLint to enforce style. I like the jsxbin example.
@runegan Do you already have an ESLint config for that?
-
@Klustre I have eslint-config-runegan, which is what the jsxbin repo is using.
-
Shall we just use that one then? My only concern is the dropping of semicolons as they are still needed in specific cases.
-
Yeah, there is some minor cases where semicolons are needed, but it should work without most of the time, at least in new versions of JS, but I'm not sure if AE/ES3 is more picky about it.
-
Account Deleted reporter For what it's worth, losing semicolons is the only thing I disagree with here.
Stylistically, the rest doesn't really matter -- but losing semicolons is the biggest thing that has the potential to actually affect the code. Telling contributors to just never use semicolons and hope that "it should work most of the time" seems unnecessarily risky / adds extra work to later find and debug.
-
I agree with @zlovatt
-
Doing a quick test it looks no semicolons mostly works on the existing codebase, at least there is no syntax errors, except for it choking when there is no semicolons after
break
inswitch..case
.I've been coding in after effects without semicolons for a while, in big and small projects, and never had any issues. There is only a few cases where semicolons are "needed" (take a look at what standardjs says about it), but those are easily avoided and usually only come up if you are writing "smart" code, so I've never had to use any.
I've not been using
switch...case
so it not working there is new to me.Because of the size of the codebase, and it already being hard to test everything, I think it might be better to leave the semicolons alone. It's a bit sad, as I think it's much nice to write and read without them, even if there is those few "issues" with it.
-
It really comes down to personal preference and habit. Much like spaces vs tabs and tab size.
I'm fine with whatever we decide, as long as ESLint is super specific, so I don't have to think about the style as it probably goes against my own style (or habit).
So with multiple people working on a codebase we'll all have different habits and the linter needs to be really verbose.
Complimentary the style should be mentioned in the contribution guide (#40)
-
-
I have run the eslint fix tool (4f165b0) and gone through and fixed trivial errors it didn't fix (a6addc8), but a few still remain.
Mostly:
aeq' was used before it was defined no-use-before-define
caused by thevar aeq = ( function () )
closures.Those closures are removed by gulp for some reason, so wondering if we need them at all.
Full list of errors: https://bitbucket.org/motiondesign/aequery/addon/pipelines/home#!/results/117
-
All linting errors and warnings are now either fixed or silenced.
The silenced ones are:
./lib/main.js 154:6 warning Blocks are nested too deeply (5) max-depth ./lib/select.js 80:14 warning Function has a complexity of 38 complexity ./lib/ui/container.js 297:13 warning Method 'addSlider' has too many parameters (5). Maximum allowed is 4 max-params
-
Great! @runegan I noticed you added some todo's in the code. We should create new issues for them.
@zlovatt @rafikhan Approved to merge and resolve this issue?
-
Account Deleted reporter Looks good!
-
Cool, guess we can merge then? @runegan
Just to be clear/for future reference: aeQuery does require semicolons!
-
@Klustre That is correct.
Merging
linting
intodevelop
and closing this issue. -
- changed status to closed
Merge branch 'linting' into develop
- linting: Rename renderqueue.unqueue_all to unqueueAll linting: Fix and silence errors Linting: Fix no-use-before-define errors lint: Update indent rules linting: Manually fix errors linting: run lint-fix Pipeline: run linter before building package Add linting rules update eslint add editorconfig Add .eslintignore Add pre-commit Add eslint and gulp lint task
closes:
#20→ <<cset b1d899839e7b>>
- Log in to comment
What style guide should be used? And how should it be enforced? My vote is for jQuery style guide and eslint for enforcing it.
I would like to modify the style slightly though. Using single quotes and not using any semicolons. Take a look at the jsxbin repo on github to see how that looks.