Page MenuHomePhabricator

Lint
ClosedPublic

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

Details

Reviewers
bengotow
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:ef90f289f4cb: Lint
Summary
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

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

max added a comment.Jul 29 2019, 3:00 AM

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

alangenfeld accepted this revision.Jul 29 2019, 3:18 PM
alangenfeld added a subscriber: sashank.

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
bengotow accepted this revision.Jul 29 2019, 3:40 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!

This revision was automatically updated to reflect the committed changes.