Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Standarize eslint - Closes #3749 #4033

Merged
merged 19 commits into from
Aug 2, 2019
Merged

Standarize eslint - Closes #3749 #4033

merged 19 commits into from
Aug 2, 2019

Conversation

shuse2
Copy link
Collaborator

@shuse2 shuse2 commented Aug 1, 2019

What was the problem?

framework had several eslint rules that is not consistent with other packages

How did I solve it?

Remove most of the custom rules.
Changed to warn level for the rules that has many usage

How to manually test it?

Test should pass (only changed the rule)

Review checklist

@@ -14,6 +14,7 @@

'use strict';

const path = require('path');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you extract path if it's only used on the branch for new relic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no global import..
I think path won't hurt to be required in the root

Copy link
Contributor

@pablitovicente pablitovicente left a comment

Choose a reason for hiding this comment

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

Minor comment but approved.

Copy link

@diego-G diego-G left a comment

Choose a reason for hiding this comment

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

Some comments to understand the motivation to disable rules in such a way. Anyways, happy to see this effort to continue the standardisation of our code! Kudos!

framework/test/mocha/common/utils/wait_for.js Show resolved Hide resolved
framework/.eslintrc.json Show resolved Hide resolved
framework/.prettierrc.json Outdated Show resolved Hide resolved
framework/.eslintrc.json Show resolved Hide resolved
framework/.eslintrc.json Show resolved Hide resolved
framework/.eslintrc.json Show resolved Hide resolved
framework/.eslintrc.json Show resolved Hide resolved
framework/.eslintrc.json Show resolved Hide resolved
@diego-G diego-G self-requested a review August 2, 2019 10:25
Copy link

@diego-G diego-G left a comment

Choose a reason for hiding this comment

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

Remember to open an issue for this comment ;) #4033 (comment)

Copy link
Contributor

@lsilvs lsilvs left a comment

Choose a reason for hiding this comment

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

lgtm

@shuse2 shuse2 merged commit a800c32 into development Aug 2, 2019
@shuse2 shuse2 deleted the 3749-standarize_eslint branch August 2, 2019 12:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove framework specific eslint rules
4 participants