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

Expand lint-fix script #25276

Merged
merged 2 commits into from
Jan 20, 2023
Merged

Expand lint-fix script #25276

merged 2 commits into from
Jan 20, 2023

Conversation

LeviPesin
Copy link
Contributor

@LeviPesin LeviPesin commented Jan 12, 2023

Related issue: #25251

Description

Expand lint test to lint everything (core, addons, examples, docs, etc) except manual.

@LeviPesin
Copy link
Contributor Author

@mrdoob @Mugen87 What's your opinion about this PR? I think it's great to also have linter reporting incorrect linting at least in addons (I think PRs break linting there quite frequently...).

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 17, 2023

I'm not 100% sure about expanding the linter like that. Originally, we wanted to enforce the code style in the core but be less strict in the examples section so it's easier for devs to contribute. Not everybody works with an IDE/editor that automatically lints the code according to the project settings. To avoid delays (and frustration) in PRs, we've often merged them as they are and cleaned things up at a later point.

@LeviPesin
Copy link
Contributor Author

Maybe then we can expand lint-fix but keep lint as it is?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 18, 2023

That sounds good to me.

@LeviPesin LeviPesin changed the title Expand lint test Expand lint-fix script Jan 20, 2023
@Mugen87 Mugen87 added this to the r149 milestone Jan 20, 2023
@Mugen87 Mugen87 merged commit 320caef into mrdoob:dev Jan 20, 2023
@LeviPesin LeviPesin deleted the patch-2 branch January 20, 2023 15:24
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.

2 participants