-
Notifications
You must be signed in to change notification settings - Fork 384
fix lint and require issues causing 'bin/rake ci' to fail #268
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
Conversation
815c91f
to
5429563
Compare
I don't know why these didn't fail on travis CI, but they failed for me locally. |
@@ -28,7 +28,7 @@ describe('CommentList', () => { | |||
id: 2, | |||
author: 'Furter', | |||
text: 'ho', | |||
}), | |||
}) |
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 we're supposed to have a trailing comma.
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.
The linter failed on the trailing comma when I ran locally, with latest code, with just a bundle and npm install and rake ci. If it doesn't for you or on travis, then there's something non-deterministic that is bleeding in from those environments or mine, and that should be locked down / prevented. I didn't dig into why, I just made this change and the linter passed.
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.
@thewoolleyman in some cases, we tried using docker for linting. I don't like that it as it doesn't seem to lock down the linter versions.
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.
OK, this one isnlt failing any more against master, but the other one is. I've removed this change.
5429563
to
bd283b2
Compare
Reviewed 1 of 2 files at r1, 1 of 1 files at r2. Comments from Reviewable |
@thewoolleyman can you rebase this on top of master? |
bd283b2
to
85606e8
Compare
@justin808 Rebased and pushed... |
Reviewed 1 of 1 files at r3. Comments from Reviewable |
Thanks @thewoolleyman Review status: Comments from Reviewable |
This change is