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

chore: Switch from tslint to eslint #1205

Merged
merged 4 commits into from Feb 2, 2022
Merged

chore: Switch from tslint to eslint #1205

merged 4 commits into from Feb 2, 2022

Conversation

fbartho
Copy link
Member

@fbartho fbartho commented Feb 1, 2022

tslint is end-of-life, and warns when you install it. Additionally, the version we were on didn't support 'import type' expressions, as well as optional-chaining & null coalescing when I tried to use those in #1204

This PR does update a bunch of tslint-disable comments, but otherwise tries to minimally change runtime source in danger-js. If we want to tighten the eslint rules, I'd be super supportive, but I didn't want to cause a ton of thrash, so mostly the rules that are enabled are the ones that don't trigger on tons of existing code.

This PR also upgrades prettier, because that was needed to handle readonly string[] arguments.

Be aware this PR drops testing support for node 10 in AppVeyor -- b66f86f @typescript-eslint doesn't support node 10

tslint is end-of-life, and warns when you install it. Additionally, the version we were on didn't support 'import type' expressions, as well as optional-chaining & null coalescing when I tried to use those in #1204

This PR does update a bunch of tslint-disable comments, but otherwise tries to minimally change runtime source in danger-js. If we want to tighten the eslint rules, I'd be super supportive, but I didn't want to cause a ton of thrash, so mostly the rules that are enabled are the ones that don't trigger on tons of existing code.
@fbartho fbartho requested a review from orta February 1, 2022 21:20
dataSent.settings.github.accessToken = "12345"

writeFileSync(pathJoin(fixtures, "bbc-dsl-input.json"), JSON.stringify(dataSent, null, " "), "utf8")
}).not.toThrow()
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻 nice work

@orta
Copy link
Member

orta commented Feb 2, 2022

Perfect PR.

WRT to tightening the linter rules, call me lightly supportive - I mostly disable eslint/tslint on repos I run nowadays because 'I know better' (aka, since I joined TS I mostly only write repos where I'm the core author and so I don't have to share well with others) so you're welcome to do it (in part because you're writing most new code in danger ATM) but know that I'm a sledgehammer who might just turn it off for sections I write :D

@orta
Copy link
Member

orta commented Feb 2, 2022

I couldn't find an email for you - but the danger slack seems to have got a bit more active over the last week or two, so you might enjoy having access too ( send me a "hey" email to the mail in my GitHub bio @orta)

@orta orta merged commit a6b40e9 into main Feb 2, 2022
@fbartho fbartho deleted the fb/eslint branch February 3, 2022 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants