-
-
Notifications
You must be signed in to change notification settings - Fork 835
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
Add typechecks, typescript coverage GH action #3136
Conversation
.github/workflows/ts-check.yml
Outdated
|
||
jobs: | ||
typecheck: | ||
name: Typecheck |
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.
Might it be worth specifying that these are Typscript typings? Just in case we opt to do some form of PHP type checking in the future.
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.
Think this is good overall.
We might want to consider implementing what you linked above. It looks quite cool and helpful!
We do want to sort those failing tests, if possible. Might be worth not actually validating typings yet, as they're always going to fail for the moment.
Is there a reason that js
files show as typed? Is it JSDoc comments? I don't think we want that, do we?
Would be nice if we could have the actions under "JS" as well so it shows something like |
Probably, but let's start with this.
We could just use
Probably type inference, mostly. I don't think there's a good/easy option to turn that off without breaking builds unfortunately.
Agreed that the GitHub actions could use standardization and cleanup. Only reason I haven't done this yet is that ideally the |
761c190
to
d261840
Compare
I set it to fail silently for now as we work to roll it out. Currently, the remaining issues are:
|
Co-authored-by: David Wheatley <hi@davwheat.dev>
fe8d6c8
to
7b81d93
Compare
Fixes #3106
Fixes #3154
Changes proposed in this pull request:
Reviewers should focus on:
How do we feel about the script names and github actions layouts?
I considered including https://github.com/codechecks/typecov but since that needs a token + additional setup, I decided not to.
Screenshot
Necessity
Confirmed
composer test
).Required changes: