-
-
Notifications
You must be signed in to change notification settings - Fork 317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
First steps towards enabling TypeScript strict mode #826
Conversation
@pavish Here's a quick beginning to the process of making our typing more strict. Making these changes across the codebase will be easy. We just need some clarity on on how to deal with null vs undefined. The diff in 1776359 will give you a sense of the kinds of changes we'll need to make. In lots of these scenarios, we can choose to resolve ambiguity towards |
@seancolsen I've responded to the discussion. My preference would be to use |
dccb04b
to
f7b4206
Compare
@pavish this is ready for review now. I kept this same PR in-place but stripped out all the complex changes, leaving only the simple ones that we should be safe to merge now, with strict mode off. I still have the rest of my changes locally and will build another PR with them later to continue working towards closing #619. |
@seancolsen Could you please update this PR to resolve the merge conflicts? I'd like to merge this in before the other current open PRs at the time of this comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seancolsen The PR looks good for the major part.
There is one major bug that needs to be fixed: Add schema modal does not open as mentioned in #826 (comment). Once this bug is fixed, we can merge this PR.
mathesar_ui/src/component-library/file-upload/FileUpload.svelte
Outdated
Show resolved
Hide resolved
The `isEditMode` is superfluous because is can be computed from the value of `AddEditSchema`.
@pavish ready for re-review |
Codecov Report
@@ Coverage Diff @@
## master #826 +/- ##
=======================================
Coverage 93.29% 93.29%
=======================================
Files 86 86
Lines 3161 3161
=======================================
Hits 2949 2949
Misses 212 212
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
@seancolsen Looks good to me. |
Works towards fixing #619 (but leaves much to be done later)
Done
undefined
instead ofnull
.Checklist
Update index.md
).master
branch of the repositoryDeveloper Certificate of Origin
Developer Certificate of Origin