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

feat: Implement new eslint-config-cozy-app (VO-541) #2104

Closed
wants to merge 1 commit into from

Conversation

acezard
Copy link
Contributor

@acezard acezard commented Mar 11, 2024

@@ -1,8 +1,9 @@
import React, { useEffect, useState } from 'react'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

those 3 removal were the only manual error fixes

@@ -113,5 +111,8 @@
"hooks": {
"pre-commit": "lint-staged"
}
},
"resolutions": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we force eslint resolution to 8 or else cozy-scripts could force a mismatch and yarn will use eslint 5 or 7 in the CLI, making the lint fail

Copy link
Member

Choose a reason for hiding this comment

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

We could make a PR on cozy-script to avoid this resolution

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could make a PR on cozy-script to avoid this resolution

@cballevre that would be preferrable for sure but I'm not sure it would be very easy to update cozy-scripts to eslint 8

@acezard acezard force-pushed the feat--Implement-new-eslint-config-cozy-app branch from 6cd1433 to 13bd5b9 Compare March 11, 2024 09:52
Copy link
Member

@cballevre cballevre left a comment

Choose a reason for hiding this comment

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

Apart from the change in the order of imports, I don't see much breaking change. It's nice not to have to re-import eslint and its plugins into the app 👍

@@ -0,0 +1,3 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

what is it used for ? 🤔

@@ -113,5 +111,8 @@
"hooks": {
"pre-commit": "lint-staged"
}
},
"resolutions": {
Copy link
Member

Choose a reason for hiding this comment

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

We could make a PR on cozy-script to avoid this resolution

@acezard acezard force-pushed the feat--Implement-new-eslint-config-cozy-app branch from 13bd5b9 to 0e050a2 Compare April 2, 2024 07:36
@acezard acezard force-pushed the feat--Implement-new-eslint-config-cozy-app branch from 0e050a2 to 5437fee Compare April 2, 2024 07:37
@acezard acezard requested a review from cballevre April 2, 2024 07:37
@acezard acezard marked this pull request as ready for review April 2, 2024 07:37
@acezard acezard requested a review from doubleface as a code owner April 2, 2024 07:37
@acezard
Copy link
Contributor Author

acezard commented Apr 23, 2024

Closed because stale

@acezard acezard closed this Apr 23, 2024
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