Skip to content
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

Fix typecheck foundations #167060

Merged
merged 23 commits into from
Sep 25, 2023
Merged

Conversation

delanni
Copy link
Contributor

@delanni delanni commented Sep 22, 2023

Summary

This PR is the core part of #166813. The original work seems to grow large, and we'd like to enable a preventive check beforehand to prevent more errors from entering the codebase.

The idea is to have a selective type check that would only check changed files' projects.

cc: @watson

@delanni delanni requested a review from a team as a code owner September 22, 2023 14:42
@delanni delanni added ci:hard-typecheck Enables full typecheck on the PR Team:Operations Team label for Operations Team release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Sep 22, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@delanni delanni removed the ci:hard-typecheck Enables full typecheck on the PR label Sep 22, 2023
package.json Outdated Show resolved Hide resolved
if (code > 0 && !(code === 143 || code === 130)) {
throw createFailError(`[${name}] exited with code ${code}`, {
exitCode: code,
});
}

// A stop signal of SIGABRT indicates a self-inflicted app crash,
// then we can't rely on an exit code, because it's not passed
if (signal === 'SIGABRT') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% confident we can trust that there will not be other signals that might indicate a crash. We don't have to do that in this PR, but in the future I'd like to simplify this code so that it just listens for the rejected promise instead of trying to be so clever and possibly missing some things

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, here's a fix: d057ecf

@@ -124,6 +124,9 @@ run(
'--pretty',
...(flagsReader.boolean('verbose') ? ['--verbose'] : []),
],
env: {
NODE_OPTIONS: '--max-old-space-size=8192',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@@ -67,7 +67,7 @@
"test:ftr:server": "node scripts/functional_tests_server",
"test:jest": "node scripts/jest",
"test:jest_integration": "node scripts/jest_integration",
"typecheck": "node scripts/type_check"
"test:type_check": "node scripts/type_check"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you rename the script?
For me test:type_check would mean run typechecks on the test code, or similar - this is not test-related, but typechecking any code

@delanni
Copy link
Contributor Author

delanni commented Sep 25, 2023

@elasticmachine merge upstream

@delanni delanni added the ci:hard-typecheck Enables full typecheck on the PR label Sep 25, 2023
@Ikuni17 Ikuni17 removed the ci:hard-typecheck Enables full typecheck on the PR label Sep 25, 2023
@Ikuni17
Copy link
Contributor

Ikuni17 commented Sep 25, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@Ikuni17 Ikuni17 merged commit e81728e into elastic:main Sep 25, 2023
Ikuni17 added a commit that referenced this pull request Sep 26, 2023
## Summary

After merging #167060, `Check Types` is going to fail in the on merge
pipeline until all type errors are triaged. For now, lets use the commit
diff type check.
@delanni delanni deleted the fix-typecheck-foundations branch September 26, 2023 08:42
watson pushed a commit to watson/kibana that referenced this pull request Sep 27, 2023
This is a manual backport of elastic#167060 + recent updates to the backported files
watson pushed a commit that referenced this pull request Sep 28, 2023
Co-authored-by: Brad White <brad.white@elastic.co>
delanni added a commit that referenced this pull request May 13, 2024
## Summary
We've recently seen a handful of step timeouts when running type-checks.
While this is not the best solution, it mitigates for potential builds
failed, and retries due to timeouts.

This PR also contains some cleanup around previous, type-check related
jobs (e.g.: the [type-check issue of 2023
august](#167060))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants