Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

DO NOT REVIEW: Prettier and research on setup via CL and Plugin#5023

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
ronaldpaek wants to merge36 commits intohackforla:gh-pages
base:gh-pages
Choose a base branch
Loading
fromronaldpaek:prettier-test

Conversation

@ronaldpaek
Copy link
Member

Fixes#4059

What changes did you make?

  • Added ESlint to GitHub Actions
  • Init root directory to node project
  • Added husky, lintsage for pre-commit
  • Added .lintstagedrc.json, .prettierigonre, .prettierrc.json, .eslintrc.json
  • Added .gitattributes file to normalize EOL settings that were in .vscode
  • scanned all files and applied the changes to them so all files will be consistent
  • made some changes to .vscode settings.json to set prettier as the default

Why did you make the changes (we will use this info to test)?

  • I was unsure, but there was no root package.json file, so I needed the root folder to be a node project to install some packages. I use npm to install prettier, eslint:prettier, husky, and stylelint
  • Initially, I only applied eslint for JS syntax rules, but I noticed similar issues. If we use prettier, we would probably be running eslint, and if we use the eslint:prettier plugin, we should avoid any conflicting issues.
  • I did not see a stylelint and package.json, but since I added a package.json file, I updated and added stylelint as a dependency to control the version along with the other packages.
  • After it worked, I saw another discussion about adding a pre-commit check. That way, if the linter spots any issue, it will warn them to fix the issues first, which should reduce unnecessary work for Pull request reviews.
  • Added the linter to GitHub actions to check js files

image

  • Here is Eslint giving us warning about some syntax errors in JS files.
    And in the terminal is prettier, saying things should be consistent and in double quotes, which will make it very fast/easy to open a file and not really even have to think but have prettier tell you to make to have consistent code with the rules you set

image

  • Here is Husky applying the pre-commit check if it doesn't pass Eslint, it will throw an error. You can always manually bypass and still commit, though, so you are not locked in.

image

  • Eslint tells us we have an undefined variable, which might not throw an error if we didn't have Eslint and could confuse less experienced developers and require more cognitive load to trace through and see what this variable is.

image

  • Ultimately, I took up this issue cause I wanted to fix the warning messages with the liquid syntax in the js file. We must separate the liquid and the js and import the result into the js file. I am still working on figuring out how to do it. Separating the data from the business logic.

@ronaldpaek
Copy link
MemberAuthor

@roslynwythe

@ronaldpaek
Copy link
MemberAuthor

There is another option we could explorehttps://antfu.me/posts/why-not-prettier. Most people like to split up linters and formatters, but we could have our linter do both if we want. Since a lot of people seem to be apprehensive about being prettier.

@ronaldpaekronaldpaek marked this pull request as draftJuly 24, 2023 17:16
@roslynwytheroslynwythe added the DraftIssue is still in the process of being created labelAug 3, 2023
@ronaldpaekronaldpaek changed the titlePrettier and research on setup via CL and PluginDO NOT REVIEW: Prettier and research on setup via CL and PluginOct 23, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

DraftIssue is still in the process of being created

Projects

Status: PR Needs review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Considering using Prettier and research on setup via CL and Plugin

2 participants

@ronaldpaek@roslynwythe

[8]ページ先頭

©2009-2025 Movatter.jp