Lint
ClosedPublic
Actions

Authored by max on Jul 25 2019, 10:17 PM.

Details

Reviewers

bengotow
alangenfeld

Group Reviewers

Restricted Project

Commits

R1:ef90f289f4cb: Lint

Summary

Resolves https://github.com/dagster-io/dagster/issues/1605

Test Plan

Unit

Diff Detail

Repository

R1 dagster

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

max created this revision.Jul 25 2019, 10:17 PM

max edited the summary of this revision. (Show Details)Jul 25 2019, 10:18 PM

Harbormaster completed remote builds in B2576: Diff 3215.Jul 25 2019, 10:25 PM

shouldn't there be a package.json change as well?

js_modules/dagit/src/runs/RunHistory.tsx
103	maybe throw instead of taking random default?

alangenfeld requested changes to this revision.Jul 26 2019, 7:58 PM

This revision now requires changes to proceed.Jul 26 2019, 7:58 PM

max added inline comments.Jul 29 2019, 2:59 AM

js_modules/dagit/src/runs/RunHistory.tsx
103	maybe, and this would certainly be a programming error if we got here, but this is still a reasonable default

@alangenfeld I'm not sure what package.json change you're thinking of here

oh - I thought this was from @sashank, we talked about getting these lint errors to show in editor by yarn add -D eslint-config-react-app + adding "react-app" to the extends key in .eslintrc.js

optional for you to do

This revision is now accepted and ready to land.Jul 29 2019, 3:18 PM

Looks good! Thanks for making all these fixes.

js_modules/dagit/src/runs/RunHistory.tsx
103	Hey! The best thing to do here would be to `assertUnreachable(sortType)` (It's a helper we added to Utils.) That uses Typescript's narrowing scopes feature to verify that by the time we reach this line the type of `sortType` is `never`, indicating that the switch is exhaustive!