Skip to content

Conversation

daviskoh
Copy link
Contributor

@daviskoh daviskoh commented Feb 6, 2015

Changed:

  • added eslint & .eslintrc (tried to unbiased judgements on proj style)
  • all specs pass & everything seems to build
  • reverted back karma runner to only run desired specs

Feedback is much appreciated!

@@ -0,0 +1,14 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried using eslint? We're moving towards using it at Facebook and would be awesome if this project would as well

@daviskoh
Copy link
Contributor Author

daviskoh commented Feb 6, 2015

Ill switch it over eslint!

* modify .eslintrc w/ globals & rules
* use judgement in deciding bet proj style & eslint recommendation
* make
@daviskoh
Copy link
Contributor Author

This was quite a messy change adding eslint and addressing the errors so I will fully understand if you decide to reject it T.T. The main comment has been updated.

@vjeux
Copy link
Contributor

vjeux commented Feb 18, 2015

I'm in vacation until the end of the week. Will check when I get back home, sorry for the wait :(

@@ -348,7 +352,7 @@ var layoutTestUtils = (function() {

var texts = {
small: 'small',
big: 'loooooooooong with space',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Facebook convention is to always put trailing comma fyi :)

@vjeux
Copy link
Contributor

vjeux commented Feb 20, 2015

So good, lots of great stuff here :) Thanks for doing this!

vjeux added a commit that referenced this pull request Feb 20, 2015
@vjeux vjeux merged commit 74a031e into facebook:master Feb 20, 2015
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.

2 participants