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.

Sign up for GitHub

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

Jump to bottom

chore: migrates from tslint to eslint #265

Closed
wants to merge 8 commits into from
Closed

chore: migrates from tslint to eslint #265

wants to merge 8 commits into from

Conversation

sosukesuzuki
Copy link
Contributor

Fixes #264

Migrates from tslint to ESLint

ESLint and eslint-plugin-react and @typescript-eslint recommended rules enabled.

The following two strict rules are disabled:

@sosukesuzuki
Install ESLint
beaa664 
Signed-off-by: Sosuke Suzuki <aosukeke@gmail.com>
@sosukesuzuki
Add .eslintrc.json
81c9f2f 
Signed-off-by: Sosuke Suzuki <aosukeke@gmail.com>
@sosukesuzuki
Follow ESLint rules
3360230 
Signed-off-by: Sosuke Suzuki <aosukeke@gmail.com>
@sosukesuzuki
Remove tslint
a3827c1 
Signed-off-by: Sosuke Suzuki <aosukeke@gmail.com>
@sosukesuzuki
Remove tslint.json
f0ba656 
Signed-off-by: Sosuke Suzuki <aosukeke@gmail.com>
@sosukesuzuki
Remove tslint.json
838b784 
Signed-off-by: Sosuke Suzuki <aosukeke@gmail.com>
@sosukesuzuki sosukesuzuki marked this pull request as ready for review September 2, 2022 11:49
@github-actions

This comment was marked as resolved.

Sign in to view
@github-actions github-actions bot added the problem/stale This has not had a response in some time label Nov 2, 2022
@github-actions github-actions bot closed this Nov 10, 2022
@agilgur5 agilgur5 reopened this Aug 16, 2023
@agilgur5 agilgur5 added type/dependencies Pull requests that update a dependency file and removed problem/stale This has not had a response in some time labels Aug 16, 2023
agilgur5
agilgur5 approved these changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@agilgur5 agilgur5 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM.

We can make further settings changes and updates as follow-ups; either way, good to move off of no longer maintained TSLint.

Need to fix some merge conflicts now though

@agilgur5 agilgur5 requested a review from rbreeze August 16, 2023 18:36
rbreeze
rbreeze approved these changes Aug 16, 2023
View reviewed changes
Copy link
Member

@rbreeze rbreeze left a comment

Choose a reason for hiding this comment

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

LGTM!

@sosukesuzuki
Merge branch 'master' into migrate-to-eslint
2dee49d
@sosukesuzuki
Fix lint problem
2bed8ae 
Signed-off-by: Sosuke Suzuki <sosuke.suzuki@dr-ubie.com>
@sosukesuzuki
Copy link
Contributor Author

Fixed conflicts

@agilgur5
Copy link
Contributor

@sosukesuzuki thanks so much for fixing the conflicts so quickly despite this PR being almost a year old! And thanks for making the contribution in the first place ❤️

@agilgur5
Copy link
Contributor

agilgur5 commented Aug 18, 2023 •
edited
Loading

Looks like the CI build is failing on incompatible Node version for one of the ESLint deps. Bumping the version CI uses to 14.17 would probably be the most minimal change to fix that

@agilgur5 agilgur5 mentioned this pull request Aug 18, 2023
Closed
@agilgur5
Copy link
Contributor

@sosukesuzuki would you like to fix the CI build as suggested above?

@sosukesuzuki
Copy link
Contributor Author

I missed your comments sorry. I'll fix build error.

@sosukesuzuki sosukesuzuki mentioned this pull request Aug 27, 2023
Closed
@github-actions

This comment was marked as resolved.

Sign in to view
@github-actions github-actions bot added the problem/stale This has not had a response in some time label Oct 23, 2023
@agilgur5 agilgur5 removed the problem/stale This has not had a response in some time label Oct 29, 2023
@agilgur5
Copy link
Contributor

agilgur5 commented Nov 3, 2023

Build error is being fixed separately as part of #432

@agilgur5 agilgur5 mentioned this pull request Nov 7, 2023
Merged
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
@github-actions github-actions bot added the problem/stale This has not had a response in some time label Jan 3, 2024
@agilgur5 agilgur5 added javascript Pull requests that update Javascript code and removed problem/stale This has not had a response in some time labels Jan 3, 2024
@agilgur5 agilgur5 mentioned this pull request Jan 6, 2024
Merged
@agilgur5
Copy link
Contributor

agilgur5 commented Jan 6, 2024

Superseded by #509. Made build/CI changes and fixed conflicts there on top of this. Credit given to OP

@agilgur5 agilgur5 closed this Jan 6, 2024
@agilgur5 agilgur5 self-assigned this Jan 6, 2024
@agilgur5 agilgur5 added the solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) label May 11, 2024
@argoproj argoproj locked as resolved and limited conversation to collaborators May 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rbreeze rbreeze rbreeze approved these changes

@agilgur5 agilgur5 agilgur5 approved these changes

Assignees

@agilgur5 agilgur5

Labels
javascript Pull requests that update Javascript code solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) type/dependencies Pull requests that update a dependency file
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate from tslint to eslint
3 participants
@sosukesuzuki @agilgur5 @rbreeze