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

chore: Revisit linting/styling #227

Open
jpmcb opened this issue Mar 19, 2024 · 8 comments
Open

chore: Revisit linting/styling #227

jpmcb opened this issue Mar 19, 2024 · 8 comments
Assignees
Labels

Comments

@jpmcb
Copy link
Member

jpmcb commented Mar 19, 2024

Maybe not blocking, but this changes a number of " double quotes over to ' single quotes: we can push this pr through but would probably be good to get the eslint/prettier checks in here along with a package.json script to easy execute linting

Originally posted by @jpmcb in #218 (comment)


Looks like we may need to revisit linting/styling in this repo (like we have in the app and etl: https://github.com/open-sauced/etl/pull/176)

Also noticed the npm run lint command is just calling next lint which we should probably call into prettier/eslint like we do elsewhere.

@Jenna59
Copy link

Jenna59 commented Apr 13, 2024

@jpmcb would this be setting up the config files for eslint and prettier? I'm interested in working on it but the pull/176 link 404d, so I'm not sure exactly what I would have to do?

@nickytonline
Copy link
Member

nickytonline commented Apr 14, 2024

@Jenna59, looking at the prettier config, the single quote change is correct.

https://github.com/open-sauced/landing-page/blob/main/.prettierrc.js#L1-L6

module.exports = {
    trailingComma: 'es5',
    tabWidth: 2,
    semi: false,
    singleQuote: true,
}

I think the issue is we need to run prettier over all the files to ensure everything is formatted correctly (step 1), and then I think we need to put in place lint-staged with husky, so we get pre-commit hooks formatting things in case people are using different editors/settings.

  "lint-staged": {
    "*.{js,jsx,ts,tsx}": "eslint --cache --fix",
-    "*.{ts,tsx}": "vitest related --run",
    "*.{js,jsx,ts,tsx,css,md}": "prettier --write"
  }

Note that the landing page doesn't use vitest, so that can be omitted.

You can look at https://github.com/open-sauced/app/blob/beta/package.json#L160-L164 for lint-staged, and it would also require setting up Husky.

I'll let others chime in though as I'm typically not working in this project.

@Jenna59
Copy link

Jenna59 commented Apr 14, 2024

.take @nickytonline would I need Sanity Studio access to work on this issue? https://github.com/open-sauced/landing-page?tab=readme-ov-file#-cms-sanity-commands

Copy link
Contributor

Thanks for taking this on! If you have not already, join the conversation in our Discord

@nickytonline
Copy link
Member

I don't think you do as I've never installed it.

@nickytonline
Copy link
Member

I would probably break this up into a couple of PRs. One for running prettier on the whole code base to fix any existing forgetting issues and a second PR which would add Husky and lint-staged.

Again though, I don't usually work in this repo, so just checking in with @brandonroberts and @zeucapua before you start any of the work.

@Jenna59
Copy link

Jenna59 commented Apr 17, 2024

Hey there @brandonroberts and @zeucapua I was hoping to start taking a look at things on Friday. Do you have any thoughts before I begin?

@jpmcb
Copy link
Member Author

jpmcb commented Aug 20, 2024

Hi @Jenna59 - still looking to tackle this? Happy to assist if you need help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants