-
-
Notifications
You must be signed in to change notification settings - Fork 30
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
Style and lint #47
Style and lint #47
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think separating out the add-tests and the reformat-templating code commits would make the diff more reviewable. In particular I was hoping to just go through the snapshot file diff and say "yeah those are all whitespace changes", but since some stuff got moved around and reordered, it wasn't as clear. I think it's good though.
Yay for linting!!
lib/constructs/interface.js
Outdated
@@ -396,7 +397,7 @@ Interface.prototype.generateSymbols = function () { | |||
if (Object.keys(unscopables).length) { | |||
this.str += ` | |||
Object.defineProperty(${this.name}.prototype, Symbol.unscopables, { | |||
value: ${JSON.stringify(unscopables, null, ' ')}, | |||
value: ${JSON.stringify(unscopables, null, " ")}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, I never knew you could do that; I always did 2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some people prefer tabs :)
52ccaba
to
e1de62d
Compare
e1de62d
to
5f7b9b9
Compare
I pulled out the test-adding commit. Now d5677bd#diff-e80cf17e46910d548417d47d60230718 should be pristine 👌 |
Note that the stringifier tests didn't work before this PR (the emitted JS isn't valid JS), and I fixed stringifier code generation in the reformat commit, only after it got too hairy to pull it out :( That was why I originally had the tests with the reformat commit. Should be all good now though. |
Fix "default": undefined
Fixes most of #35, doesn't use prettier-eslint yet as I don't think that's necessary.