Skip to content

Conversation

@chicoxyzzy
Copy link
Contributor

No description provided.

@zpao
Copy link
Member

zpao commented Feb 11, 2015

I totally get where you're coming from but let's leave this for now. We (FB) still use jshint internally so these actually help keep things a bit quieter there. I'd be fine dropping the jshint things from everything not in src/ and making sure we use eslint there (we don't currently), if you want to go down that route.

@chicoxyzzy chicoxyzzy changed the title completely remove jshint ESLint coverage Feb 12, 2015
@chicoxyzzy
Copy link
Contributor Author

@zpao now ESLint is everywhere except src/. That was not easy 😩

Copy link
Member

Choose a reason for hiding this comment

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

Can you revert these changes. This is intended (to match the formatting of the license as it appears in regular source files).

@zpao
Copy link
Member

zpao commented Feb 12, 2015

Overall, herculean effort. Thanks! Just a few smaller things inline I think.

Also, it looks like something got weird with your branching (or GitHub broke). Can you rebase and whatever you need to get that sorted out.

@chicoxyzzy
Copy link
Contributor Author

Also, it looks like something got weird with your branching (or GitHub broke). Can you rebase and whatever you need to get that sorted out.

Wow.. it's really weird. I'll check it

@chicoxyzzy chicoxyzzy force-pushed the remove-jshint branch 2 times, most recently from 512c90c to 2c0f257 Compare February 12, 2015 21:04
@chicoxyzzy
Copy link
Contributor Author

@zpao rebased

@chicoxyzzy
Copy link
Contributor Author

@zpao also just noticed you updated /src/.eslintrc in #3133 but it's better to add this rules to /.eslintrc (if this PR will be merged)

@chicoxyzzy
Copy link
Contributor Author

about commit weirdness. I was playing with this script trying to change my name from chico to my real name but I thought I was playing in my test repo and so I broke git history. Then I pushed my broken react branch to my remote next day. So I solved that by -rm -rf my .git directory, reinit git and rebase my branch from master to apply my changes on top of it.

@zpao
Copy link
Member

zpao commented Feb 13, 2015

Alright, lets do it. Thanks for fixing the commit weirdness!

This will definitely be useful and I know it was painstaking work, so we really appreciate it.

zpao added a commit that referenced this pull request Feb 13, 2015
@zpao zpao merged commit 2a3f431 into facebook:master Feb 13, 2015
@chicoxyzzy chicoxyzzy deleted the remove-jshint branch February 13, 2015 20:26
@chicoxyzzy chicoxyzzy mentioned this pull request 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.

3 participants