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

Migrate to latest ESLint Version #153

Merged
merged 3 commits into from
Sep 3, 2024

Conversation

dmanto
Copy link
Contributor

@dmanto dmanto commented Aug 31, 2024

Update: Migrate to Latest ESLint Version

Description:

This PR introduces an upgrade to the latest version of ESLint. While there are multiple areas to review, I believe this is a solid starting point for improving code quality and consistency across the project.

Summary of Changes:

  • Commit 1:

    • Separated Prettier from ESLint for better clarity and maintainability.
    • Migrated the configuration files to accommodate the latest ESLint version.
  • Commit 2:

    • Resolved code issues based on the new ESLint rules.
    • Note: Some rules may appear stricter than before. This is something we should evaluate and adjust as necessary.

Next Steps:

  • Review the updated rules to ensure they align with our coding standards.
  • Provide feedback on any rules that may need tweaking or further discussion.

Comments and suggestions are welcome!

eslint.config.js Outdated
// Add the files for applying the recommended TypeScript configs
// only for the Typescript files.
// This is necessary when we have the multiple extensions files
// (e.g. .ts, .tsx, .js, .cjs, .mjs, etc.).
Copy link
Member

Choose a reason for hiding this comment

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

Just remove the comments from the file, the object names are descriptive enough.

@kraih
Copy link
Member

kraih commented Aug 31, 2024

Guess that means we need a separate workflow to verify prettier formatting?

@dmanto
Copy link
Contributor Author

dmanto commented Aug 31, 2024

Guess that means we need a separate workflow to verify prettier formatting?

I can rename the existing prelint and prelint:fix scripts to format and format:fix to handle Prettier separately. Alternatively, I could keep the current names, which would avoid the need to run the two steps manually.

On second thought, prelint and prelint:fix could just call format and format:fix so we keep things DRY. Which approach do you prefer?

@kraih
Copy link
Member

kraih commented Sep 1, 2024

Guess that means we need a separate workflow to verify prettier formatting?

I can rename the existing prelint and prelint:fix scripts to format and format:fix to handle Prettier separately. Alternatively, I could keep the current names, which would avoid the need to run the two steps manually.

On second thought, prelint and prelint:fix could just call format and format:fix so we keep things DRY. Which approach do you prefer?

Afraid i have no idea what you're referring to. As far as i understand prettier used to be called as part of the eslint GitHub Action. Now that prettier is not in the eslint config anymore what will call it in CI?

@dmanto
Copy link
Contributor Author

dmanto commented Sep 1, 2024

Guess that means we need a separate workflow to verify prettier formatting?

I can rename the existing prelint and prelint:fix scripts to format and format:fix to handle Prettier separately. Alternatively, I could keep the current names, which would avoid the need to run the two steps manually.
On second thought, prelint and prelint:fix could just call format and format:fix so we keep things DRY. Which approach do you prefer?

Afraid i have no idea what you're referring to. As far as i understand prettier used to be called as part of the eslint GitHub Action. Now that prettier is not in the eslint config anymore what will call it in CI?

Prettier will still run as part of the CI process because I added pre scripts for both lint and lint:fix (prelint and prelint:fix). npm automatically runs these pre scripts before executing the corresponding lint and lint:fix scripts. So, Prettier will be invoked every time linting occurs, both in local development and CI.

@kraih kraih merged commit 1f1f60c into mojolicious:main Sep 3, 2024
9 of 10 checks passed
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