General Jest padding rule

dangreenisrael / eslint-plugin-jest-formatting

ESLint rules for formatting test suites written for jest.

MIT License

156 stars 13 forks source link

General Jest padding rule #44

Closed benkimpel closed 5 years ago

benkimpel commented 5 years ago

The code in rules/padding.ts is based on the ESLint rule padding-line-between-statement. It's been altered to look for Jest tokens in expressions.

Added rules and test for afterAll, afterEach, beforeAll, beforeEach, describe, expect, it/test, and all tokens together.

Additional changes

Pointed package.json dev dependency for this package to itself and set up linking
Tweak to .gitignore to make lib absolute. (It was ignoring my new tests in test/lib.)

benkimpel commented 5 years ago

Working on CI

benkimpel commented 5 years ago

Haha... I mean I'm working on fixing CI.

benkimpel commented 5 years ago

Sorry if I'm spamming CircleCI failures. There's something strange going on. If I SSH into the CircleCI box with "Rebuild job with SSH" I'm able to run yarn lint successfully. It's weird.

benkimpel commented 5 years ago

~~Ohhhhhhhhhh. I think I got it. I think it has something to do with how Circle is running commands. sh -c /home/circleci/repo/node_modules/.bin/eslint .~~

Nope, just needed to link it

benkimpel commented 5 years ago

A glorious green check mark. That's why we're programmers, amirite? For the check marks.

benkimpel commented 5 years ago

Ok, @dangreenisrael. I think this PR is feature complete now. I'll clean up the commit message and add comments on the code on Monday. There are a few items that I'll call your attention to in particular... just to make sure they're not missed in review since they're minor changes.

And all green on CI and I've run it against my own web app with good results.

dangreenisrael commented 5 years ago

@benkimpel Starting the CR now. Thanks so much for doing this, and for your patience with my delayed code reviews.

dangreenisrael commented 5 years ago

Regarding all the request for types: I generally consider all of them as nice to haves. I only recently converted the project to typescript and a lot of the existing stuff didn't have types to begin with so I would be quite a hypocrite to insist that all of your (awesome) stuff is fully typed.

That said, if you could add them that would be awesome, but if you don't want for some of them - feel free to just note that in the comments and mark the comments as resolved

dangreenisrael commented 5 years ago

Once this is all ready to go, one of us can update the main README

benkimpel commented 5 years ago

@dangreenisrael Great feedback. I'm on it (starting tomorrow).

Good thinking on the README... I totally forgot.

Type requests are fine by me!

benkimpel commented 5 years ago

All right. I think I'll have a final commit tomorrow for last bit of clean up and type info. Exciting. I'll re-request a review when ready.

benkimpel commented 5 years ago

Ready for another look @dangreenisrael!

benkimpel commented 5 years ago

All right. I think I'll have a final commit tomorrow for last bit of clean up and type info. Exciting. I'll re-request a review when ready. -- Ben Kimpel (4 days ago)

"tommorow"... heh. 4-days-ago-Ben so optimistic... so naive.

Anyway, if this gets the thumbs up I'll squash and merge (if that's your preferred workflow) and make a nice commit message. Or you can! LMK.

benkimpel commented 5 years ago

All right, @dangreenisrael. Squished down to 5 solid commits with good explanations. Merge whenever you're ready.

Removed the version bump in package.json and will leave it up to you to PR that change.

dangreenisrael commented 5 years ago

@benkimpel LGTM!

Would you like the honours of hitting the big green button?

*Merge commit please - those are some nice commit messages!!!