csdojo-defaang / defaang

A website that will curate recently-asked interview questions from FAANG+. Currently inactive. Check out: https://github.com/ykdojo/OpenStream
MIT License
509 stars 120 forks source link

dx: add .editorconfig #79

Closed mark-omarov closed 2 years ago

mark-omarov commented 2 years ago

πŸ‘¨β€πŸ’» Changes proposed

πŸ“„ Note to reviewers

This ensures we don't enforce usage of VSCode in the project. Editorconfig is accepted by many code editors and IDEs out of the box.

vercel[bot] commented 2 years ago

Someone is attempting to deploy a commit to a Personal Account owned by @ykdojo on Vercel.

@ykdojo first needs to authorize it.

iShibi commented 2 years ago

It seems you have not installed the required devDependencies to auto fix formatting and check for commit messages. Follow the following steps to pass the checks:

  1. npm i
  2. npm run format:fix
  3. git add .
  4. git commit -m "<conventional commit message here>"
mark-omarov commented 2 years ago

It seems you have not installed the required devDependencies to auto fix formatting and check for commit messages. Follow the following steps to pass the checks:

  1. npm i
  2. npm run format:fix
  3. git add .
  4. git commit -m "<conventional commit message here>"

Weird. Husky is running a pre-commit hook that triggers lint-staged to execute eslint fix and format by prettier, that all works just fine for me.

I ran it manually to ensure it's correctly formatted, here's the result:

❯ npm run format:check

> format:check
> prettier . --check

Checking formatting...
All matched files use Prettier code style!

I've checked the failed workflow, and it's failing for files that I haven't touched by this PR, which is weird.

2022-08-15T01:31:08.2186559Z > format:check
2022-08-15T01:31:08.2187861Z > prettier . --check
2022-08-15T01:31:08.2192886Z 
2022-08-15T01:31:08.9244300Z Checking formatting...
2022-08-15T01:31:09.0216788Z [warn] .commitlintrc.json
2022-08-15T01:31:09.0293953Z [warn] .eslintrc.json
2022-08-15T01:31:09.1965791Z [warn] .lintstagedrc.json
2022-08-15T01:31:09.2417227Z [warn] .vscode/extensions.json
2022-08-15T01:31:09.2517337Z [warn] .vscode/settings.json
2022-08-15T01:31:09.4899193Z [warn] next.config.js
2022-08-15T01:31:09.7950454Z [warn] package-lock.json
2022-08-15T01:31:09.8445208Z [warn] package.json
2022-08-15T01:31:10.0813055Z [warn] pages/_app.tsx
2022-08-15T01:31:10.1117941Z [warn] pages/_document.tsx
2022-08-15T01:31:10.1339527Z [warn] pages/api/hello.ts
2022-08-15T01:31:10.1630566Z [warn] pages/index.tsx
2022-08-15T01:31:10.2148618Z [warn] pages/signin.tsx
2022-08-15T01:31:10.2476786Z [warn] pages/signup.tsx
2022-08-15T01:31:10.2655736Z [warn] postcss.config.js
2022-08-15T01:31:10.3462208Z [warn] tailwind.config.js
2022-08-15T01:31:10.3540947Z [warn] tsconfig.json
2022-08-15T01:31:10.3663652Z [warn] Code style issues found in 17 files. Forgot to run Prettier?

As per your suggestion, I've also tried to auto-fix it with format:fix, no files were formatted. Could you please restart the workflow for this failed step?

iShibi commented 2 years ago

We had a big change of tabs -> spaces recently and that might be a reason for it. Wait for the following PR to get merged unitl then I'll see what else could be the reason:

mark-omarov commented 2 years ago

We had a big change of tabs -> spaces recently and that might be a reason for it. Wait for the following PR to get merged unitl then I'll see what else could be the reason:

I see, if currently spaces are used that could be a reason for breaking since I've configured to use tabs, the same way as the current vscode configuration file is configured (which I used to match the project configuration with). I don't think tabs are particularly better or worse than spaces, but I can't care less - whatever the project is using. Since I can't restart the job myself, I'll ping you once the blocking PR is merged, if you merge that PR yourself would be nice if you could restart the job as well. Thanks!

mark-omarov commented 2 years ago

@ykdojo please do not merge just yet, once we deal with the failing CI step, probably once the #80 is merged - we can proceed. Thanks!

iShibi commented 2 years ago

I reran the job after merging #80 but the prettier job is still failing. I'm going out right now, will look into it once I'm back home.

iShibi commented 2 years ago

@mark-omarov Rebasing the PR with main fixed the issue. Do you have anything else to add here or should we merge?

mark-omarov commented 2 years ago

@mark-omarov Rebasing the PR with main fixed the issue. Do you have anything else to add here or should we merge?

Nothing else, good to merge πŸ‘