-
Notifications
You must be signed in to change notification settings - Fork 152
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
chore(null): Add CI task to check for new files to add to tsconfig.strictNullChecks.json #5918
Conversation
To ensure these files are included: | ||
1) Pull this branch | ||
2) Run "yarn null:autoadd" | ||
3) Commit and push the resulting change to tsconfig.strictNullChecks.json |
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.
I'm confused by this logic- the comment at the top of the file says that we're verifying that there are NO changes to tsconfig.strictNullChecks.json, but this error message suggests that we DO want to update that file. What is the actual expected outcome?
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.
The expected outcome is that CI will pass if this finds no changes and will fail if it does, prompting the author to run it and add those changes manually. This workflow is based on the behavior we use in accessibility-insights-for-android-service to ensure that gradle.lock is kept up-to-date
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.
Ah, okay, makes sense to me!
To ensure these files are included: | ||
1) Pull this branch | ||
2) Run "yarn null:autoadd" | ||
3) Commit and push the resulting change to tsconfig.strictNullChecks.json |
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.
Ah, okay, makes sense to me!
Details
Add an additional task to the
lints
CI job to runyarn null:autoadd
and determine if there are new files to be added totsconfig.strictNullChecks.json
for the PR. If it fails, it will display an error message reminding the author to runyarn null:autoadd
and commit the results.Motivation
This will help us work towards #2869 as we add/change files. As things are now, if someone wants to focus on adding some files to
tsconfig.strictNullChecks.json
in a dedicated PR, they often find a whole host of additional files that could have been added earlier. This PR ensures we expand our strict null checks whenever there are new files to include.Context
Another approach we identified for keeping
tsconfig.strictNullChecks.json
up to date involves flippingtsconfig.strictNullChecks.json
to explicitlyexclude
files instead of usinginclude
, thus automatically including new files. This would require adjusting all of our strict null scripts (e.g., autoadd, find, etc.) to follow that convention, which is not an insignificant amount of work.Pull request checklist
yarn fastpass
yarn test
)<rootDir>/test-results/unit/coverage
fix:
,chore:
,feat(feature-name):
,refactor:
). SeeCONTRIBUTING.md
.