aaronpowell / db.js

db.js is a wrapper for IndexedDB to make it easier to work against
http://aaronpowell.github.com/db.js/
MIT License
818 stars 142 forks source link

Add onabort for all transactions; expose deleteDatabase success event… #148

Closed brettz9 closed 8 years ago

brettz9 commented 8 years ago

…; add ES6-based rules which tighten up code; add a few other ESLint rules which are not in "standard" but which are good practices we are already following.

A couple other ES6 rules you might want to add, but which I didn't add as I didn't know if you wanted the hassle of having to adhere to them: no-var, prefer-const, prefer-reflect, prefer-template, prefer-arrow-callback, object-shorthand, no-arrow-condition, arrow-body-style, and arrow-parens. (The ones I did add, however, I think are pretty clearly beneficial.)

aaronpowell commented 8 years ago

I see the ESLint rules and transaction rejections as two separate unrelated pieces of work.

I don't quite see why some of these rules need to be added/changed as I want to stick to the standard rules as described here.

brettz9 commented 8 years ago

Because you deviated from the semi-colon practice of standard, the no-extra-semi seems a useful accompaniment.

The following enforce very common practice across JavaScript projects (like camelcase) or useful practices which prevent bad or incorrect code (e.g., no-restricted-syntax). I really didn't see any plus to not including them; they are not really deviations from standard's style approach, except that they add some strictness.

+    "camelcase": 2,
+    "no-restricted-syntax": [2, "WithStatement"],
+    "no-constant-condition": 2,
+    "no-empty": 2,
+    "valid-jsdoc": 2,

The following are all ES6-only and apparently the reason they are not yet added to standard. I did not include those ES6 items which seemed unnecessary or inconsistent with your current styling.

+    "arrow-spacing": 2,
+    "constructor-super": 2,
+    "generator-star-spacing": 2,
+    "no-class-assign": 2,
+    "no-const-assign": 2,
+    "no-dupe-class-members": 2,
+    "no-this-before-super": 2,
+    "prefer-spread": 2,
+    "require-yield": 2

I'm happy to revert them, but I didn't add these particular ones arbitrarily. Let me know if you want all or just some removed from this PR.

aaronpowell commented 8 years ago

The reason I question them being there is a) I don't think anything is violating them at the moment and b) I prefer to avoid things for the sake of having them there. So I'd prefer to leave them out until such time as they'd actually be needed

brettz9 commented 8 years ago

Ok, sure. My philosophy is that the purpose of linting is to catch mistakes before they happen rather than merely for enforcing style. But it's your project; I'll remove them.

brettz9 commented 8 years ago

I have reverted the ESLint changes