Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 14, 2022
Merged

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

merged 2 commits into from
Mar 14, 2022
+5 −3

Conversation

greyscaled
Copy link
Contributor

@greyscaled greyscaled commented Mar 13, 2022 •
edited
Loading

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 requested a review from bryphe-coder March 13, 2022 21:20
@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
kylecarbs
kylecarbs approved these changes Mar 13, 2022
View reviewed changes
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 •
edited
Loading

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.

@greyscaled
chore: configure eslint vs code plugin
13e35e2 
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 greyscaled force-pushed the vapurrmaid/fix-eslint branch from d9da0f3 to 13e35e2 Compare March 13, 2022 21:30
@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 mentioned this pull request Mar 13, 2022
Closed
@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 •
edited
Loading

@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
Reviewers

@kylecarbs kylecarbs kylecarbs approved these changes

@bryphe-coder bryphe-coder Awaiting requested review from bryphe-coder

Assignees

@greyscaled greyscaled

Labels
None yet
Projects
None yet
Milestone
V2 Beta
Development

Successfully merging this pull request may close these issues.
3 participants
@greyscaled @kylecarbs @misskniss