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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(site): vscode eslint plugin working directory #423

Merged
merged 2 commits into from
Mar 14, 2022

Conversation

greyscaled
Copy link
Contributor

@greyscaled greyscaled commented Mar 13, 2022

Summary

The .eslintrc.yaml is located in site, rather than the root of the repository. The VS Code plugin has a CWD of the repository root, so it looks for tsconfig.test.json in the root rather in site (resolving "./tsconfig.test.json")

Impact of Change

No more red squiggles in VS Code on every open TypeScript (ts, tsx) file in site. I verified that the VS Code ESLint Plugin corrected code automatically (and applied yellow and red squiggles as appropriate). This may not have worked identically before, so we should see an improvement to the TS development experience in site 馃帀 馃帄 .

See Also

Personal Note

This is my first primary author commit in this repo!

@greyscaled greyscaled self-assigned this Mar 13, 2022
@greyscaled greyscaled changed the title fix(site): .eslintrc.yaml tsconfigRootDir fix(site): eslint tsconfigRootDir Mar 13, 2022
Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

Great fix! Thanks @vapurrmaid 馃コ

@greyscaled
Copy link
Contributor Author

ah crap, this breaks lint scripts...so it fixed one thing but broke another 馃槀. I will look into this!

@codecov
Copy link

codecov bot commented Mar 13, 2022

Codecov Report

Merging #423 (045e202) into main (6c83907) will increase coverage by 0.97%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #423      +/-   ##
==========================================
+ Coverage   67.19%   68.16%   +0.97%     
==========================================
  Files         155      159       +4     
  Lines        9044     9226     +182     
  Branches       73       73              
==========================================
+ Hits         6077     6289     +212     
+ Misses       2340     2316      -24     
+ Partials      627      621       -6     
Flag Coverage 螖
unittest-go-macos-latest 62.84% <酶> (?)
unittest-go-ubuntu-latest 67.59% <酶> (+0.09%) 猬嗭笍
unittest-go-windows-2022 62.15% <酶> (?)
unittest-js 64.07% <酶> (酶)
Impacted Files Coverage 螖
pty/start_windows.go 60.86% <0.00%> (酶)
pty/pty_windows.go 68.25% <0.00%> (酶)
agent/usershell/usershell_darwin.go 0.00% <0.00%> (酶)
agent/usershell/usershell_windows.go 0.00% <0.00%> (酶)
coderd/provisionerdaemons.go 57.06% <0.00%> (+0.54%) 猬嗭笍
coderd/users.go 61.53% <0.00%> (+0.54%) 猬嗭笍
peer/channel.go 83.62% <0.00%> (+0.58%) 猬嗭笍
codersdk/users.go 68.42% <0.00%> (+1.75%) 猬嗭笍
coderd/parameter/compute.go 74.07% <0.00%> (+2.22%) 猬嗭笍
coderd/parameters.go 65.38% <0.00%> (+2.30%) 猬嗭笍
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 6c83907...045e202. Read the comment docs.

Summary:

The .eslintrc.yaml is located in site, rather than the root of the
repository. The VS Code plugin has a CWD of the repository root, so it
looks for tsconfig.test.json in the root rather in site.

See Also:

- microsoft/vscode-eslint#196
- typescript-eslint/typescript-eslint#251
@greyscaled
Copy link
Contributor Author

@kylecarbs Now that I realize the errors were specific to VS Code plugin, I just had to tell it where our site project was (verified by researching the plugin issues). 馃帀

@greyscaled greyscaled changed the title fix(site): eslint tsconfigRootDir fix(site): vscode eslint plugin working directory Mar 13, 2022
@greyscaled
Copy link
Contributor Author

@kylecarbs I'm not authorized to merge the PR 馃槀. Are you able to set something up for me to be able to merge?

@kylecarbs
Copy link
Member

Oh... hahahah yes I'll fix now!

@kylecarbs
Copy link
Member

@vapurrmaid is there a message that displays? I'm not sure why merge would be blocked.

@greyscaled
Copy link
Contributor Author

@vapurrmaid is there a message that displays? I'm not sure why merge would be blocked.

Yea! Here's a screenshot and a link to that doc

authorize

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches

@kylecarbs
Copy link
Member

@vapurrmaid does it work now?

@greyscaled
Copy link
Contributor Author

greyscaled commented Mar 13, 2022

@vapurrmaid does it work now?

Yes 馃檶馃徎 , thank you 馃帄 !

I have now officially contributed (as a primary commit author) to v2, very exciting 馃帀 馃帀

@greyscaled greyscaled merged commit 60f3cf6 into main Mar 14, 2022
@greyscaled greyscaled deleted the vapurrmaid/fix-eslint branch March 14, 2022 00:02
@misskniss misskniss added this to the V2 Beta milestone May 15, 2022
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.

None yet

3 participants