search

eslint / eslint-transforms

Codemods for the ESLint ecosystem
Other
22 stars 8 forks source link

add new-rule-format transform and initial repo structure #1

Closed vitorbal closed 8 years ago

vitorbal commented 8 years ago

I ran the transform on our own core rules one more time just to make sure everything was fine, and I also added some tests here.

Another thing to note is that I found a couple of eslint-plugins in the wild that use ES6 module syntax for their rules, which the transform does not support currently.

ilyavolodin commented 8 years ago

Maybe it's worth mentioning in the Readme file that those codemods target ES5 rules, and will not work for ES6? Other than that, looks good to me, thanks!

nzakas commented 8 years ago

Agreed. If there are some rules in the wild that are using ES6, we should make it clear that this only works on ES5 rules.

vitorbal commented 8 years ago

@ilyavolodin @nzakas makes sense! I'll add a note.

I'm working on making the transform run for ES6 rules as well, I'll try to sneak that in before the announcement.

vitorbal commented 8 years ago

Added a note.

ilyavolodin commented 8 years ago

LGTM

nzakas commented 8 years ago

Ooh, I just realized something: if we don't publish this to npm, then someone could grab this name. Maybe a simple CLI would be good to include? (Also helps with versioning.)

vitorbal commented 8 years ago

I pushed a simple CLI, added eslint-release for publishing and updated the README. Let me know what you think!

nzakas commented 8 years ago

Looks like a good start! Can I ask a favor? We try to keep all the repos using the same basic setup and tools so people can easily move between repos to make changes. I noticed you're using ES6 and Jest, which is very different from our other projects. Would you mind switching to our standard ES5 and Mocha/Chai setup?

Also, we keep tests in a top level directory called tests and test fixtures in tests/fixtures, would you mind moving the tests and fixtures?

vitorbal commented 8 years ago

@nzakas Yes, absolutely! I will work on it this week. Sorry for the delayed response, i was out of the country for the weekend :)

The only thing i wonder is if migrating away from Jest will be a big hassle, since I'm using the test utils provided by jscodeshift to test transforms, and they were designed to work with Jest. I'll have to check if we can plug it into a Mocha/Chai setup On Fri, May 27, 2016 at 6:00 PM Nicholas C. Zakas notifications@github.com wrote:

Looks like a good start! Can I ask a favor? We try to keep all the repos using the same basic setup and tools so people can easily move between repos to make changes. I noticed you're using ES6 and Jest, which is very different from our other projects. Would you mind switching to our standard ES5 and Mocha/Chai setup?

Also, we keep tests in a top level directory called tests and test fixtures in tests/fixtures, would you mind moving the tests and fixtures?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/eslint/eslint-transforms/pull/1#issuecomment-222185522, or mute the thread https://github.com/notifications/unsubscribe/AAmNdpDG2c9ovgwP6OjRN9tRJpCfwW0Bks5qFxUsgaJpZM4IhtXT .

vitorbal commented 8 years ago

@ilyavolodin @nzakas Refactored everything to use ES5 only, and changed the tests to use Mocha/Chai instead of Jest. Let me know what you think!

nzakas commented 8 years ago

Thanks! Just noticed a couple minor docs things. Otherwise, lgtm.

vitorbal commented 8 years ago

Thanks again for the feedback! Took care of everything in the latest commit.

nzakas commented 8 years ago

Great, I think this is a strong starting point. We can handle anything else that comes up as bugs.