Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standard js style guide (WiP) #177

Open
wants to merge 56 commits into
base: master
Choose a base branch
from
Open

Conversation

quadrophobiac
Copy link
Collaborator

This is to fix #175

Including standard has actually flagged up a lot of errors within Comma Chameleon, which is obviously a good thing but also why a WiP label is attached to this PR. Will use this PR as a place from which to list the relevant bugs that have been encountered

Comma Chameleon has a gh-pages served site in its docs folder that we
are going to ignore for now
Exec *is* required in order that the build package terminates
other electron applications adhere to const instead of globals,
commonly declaring a group of them together like so `const {Menu, app,
BrowserWindow, dialog} = require('electron')
`
This is a HACK and will not persist in the long run, its technically
abusing a feature of (standard
js)[https://www.npmjs.com/package/standard#i-use-a-library-that-pollutes
-the-global-namespace-how-do-i-prevent-variable-is-not-defined-errors].
Doing so is warranted as adhering to standard JS will involve a
refactor of code logic which I deem better addressed in its own ticket
This is a noted problem in the standard documentation relating to how
mocha works as a test suite
basically catching jQuery and the HandsOnTable object
StandardJS mandates path.join but Electron stipulation of interpolation
also passes muster
https://github.com/electron/electron/blob/master/docs/api/browser-window
.md
Standard requires errors to be thrown
this seems a suspect/shallow way of catching errors, but the usual way
of `.on(err, //etc)` doesn’t satisfy standards so including this way to
satisfy standardJS
Including only to satisfy StandardJS - I’m really not sure that errors
need to be caught during specs but happy to be corrected by someone
more au fait with this
this may or may not be worth keeping
let statement declares a block scope local variable, which can then be
modified in mocha `beforeEach` and still pass standardJS standards
There were two vars called hot up until this point
I suspect this spec (line 25 - 35) needs a refactor
@quadrophobiac
Copy link
Collaborator Author

coverage WILL drop for this as I've pruned some non useful tests

not sure if this will work but making the amendment so this PR is
consistent all the way around
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include StandardJS Javascript Style Guide
1 participant