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

Update eslint to the latest version #8770

Merged
merged 1 commit into from
Nov 23, 2020
Merged

Update eslint to the latest version #8770

merged 1 commit into from
Nov 23, 2020

Conversation

benoitf
Copy link
Contributor

@benoitf benoitf commented Nov 23, 2020

What it does

Update eslint to the latest version
Need to rename no-shadow to @typescript-eslint/no-shadow as specified there
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-shadow.md

How to test

build should be working as I changed only comments in source code

Review checklist

Reminder for reviewers

Change-Id: Ib864501e48772b224b4999718f4f8dfbeb8964d6
Signed-off-by: Florent Benoit fbenoit@redhat.com

Need to rename no-shadow to @typescript-eslint/no-shadow as specified there:
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-shadow.md

Change-Id: Ib864501e48772b224b4999718f4f8dfbeb8964d6
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@vince-fugnitto vince-fugnitto added dependencies pull requests that update a dependency file linting issues related to linting labels Nov 23, 2020
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I verified the following 👍

  • I confirmed that the new dependencies are license compatible following the guidelines.
  • I confirmed that the new rule works correctly, and pre-existing rules also work with the new eslint update.

@benoitf
Copy link
Contributor Author

benoitf commented Nov 23, 2020

@vince-fugnitto should I wait somehow to have travis-ci back ?

@vince-fugnitto
Copy link
Member

@vince-fugnitto should I wait somehow to have travis-ci back ?

@benoitf I build the pull-request locally, and ran the tests (at least on Linux).
I'm not sure how long it will take to have Travis back (since we ran out of credits), it is likely we will need to replace it with GitHub actions. However, if you'd like to be safe we can bring it up in the dev meeting if you think we should hold off pull-requests till the release.

@benoitf
Copy link
Contributor Author

benoitf commented Nov 23, 2020

so we've successful builds on linux (your case) and macos (my case) so I would say we're fine to merge.

@vince-fugnitto
Copy link
Member

so we've successful builds on linux (your case) and macos (my case) so I would say we're fine to merge.

I believe so, I can have a colleague test on windows if you'd like?

@benoitf
Copy link
Contributor Author

benoitf commented Nov 23, 2020

@vince-fugnitto that would be perfect

@vince-fugnitto
Copy link
Member

@vince-fugnitto that would be perfect

@dukengn do you mind confirming the changes work correctly on windows? (since there is no CI at the moment)

Copy link
Contributor

@DucNgn DucNgn left a comment

Choose a reason for hiding this comment

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

I have tested on Windows and confirmed that the new rules work great and the existing rules are also working well with the update 👍

@benoitf benoitf merged commit 6ef0867 into master Nov 23, 2020
@github-actions github-actions bot added this to the 1.8.0 milestone Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file linting issues related to linting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants