-
Notifications
You must be signed in to change notification settings - Fork 148
refactor node-utils type guards to use the AST_NODE_TYPES enum #205
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
refactor node-utils type guards to use the AST_NODE_TYPES enum #205
Conversation
Signed-off-by: Josh Kelly <[email protected]>
Signed-off-by: Josh Kelly <[email protected]>
Signed-off-by: Josh Kelly <[email protected]>
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.
Awesome, thanks for your first contribution!
@all-contributors please add @codecog for code. |
I've put up a pull request to add @codecog! 🎉 |
hey @Belco90 what do you think about adding a ticket to check the stylings in Travis? I had hoped that those no styling issues would be caught on CI but I think we're not checking anything like that |
@gndelia you mean for running a "code validation" step in the CI before running the tests? |
Ideally it would be more of a "precommit" step imo - formatting and linting issues should be addressed before CI runs. |
yes. Just checking the formatting + eslint are correct
true, but that can be skipped ( btw thank you for your first PR 🎉 |
Yeah, we already have the pre-commit hook enabled but has @gndelia pointed it can be skipped (could be wrongly setup too? I just checked the config and I have a slightly different one on other projects). Running code validation in the CI will make sure everyone is applying the proper formatting/linting but without modifying anything. I'm happy to have a task to add that, and check if the pre-commit is working fine. |
For me the pre-commit is working on (on windows). |
The precommit didn’t work for me other than the commit message. Lint errors
and formatting weren’t picked up.
…On Mon, 3 Aug 2020 at 17:58, Tim Deschryver ***@***.***> wrote:
For me the pre-commit is working on (on windows).
But it can always be skipped (with --skipValidation), 👍 for CI.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#205 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABG6THEUAXPYRDVJSWSVUP3R63UCFANCNFSM4PTNJBVQ>
.
|
Per our setup it will format only those files modified (staged files). Bear that in mind. |
I opened #210 |
🎉 This PR is included in version 3.4.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This will migrate all of the hardcoded type guards to use the ENUM provided by
@typescript-eslint/experimental-utils
. There is also a couple of minor diffs to formatting after runningnpm run format
and to fix some linting errors.