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

Set up knip for dead code / dependency detection #225

Merged
merged 9 commits into from Jun 30, 2023
Merged

Set up knip for dead code / dependency detection #225

merged 9 commits into from Jun 30, 2023

Conversation

danvk
Copy link
Collaborator

@danvk danvk commented Jun 29, 2023

Knip is a new tool I've been meaning to try out for detecting unused code and dependencies in JS/TS projects. It seems pretty low configuration and comes with a bunch of plugins for tracking dependencies around particular tools, e.g. vitest and prettier.

For an example of what a failure looks like, see https://github.com/refstudio/refstudio/actions/runs/5414908438/jobs/9842668221?pr=225

@codecov
Copy link

codecov bot commented Jun 29, 2023

Codecov Report

Merging #225 (74f16a6) into main (a15b30f) will increase coverage by 0.45%.
The diff coverage is 58.06%.

@@            Coverage Diff             @@
##             main     #225      +/-   ##
==========================================
+ Coverage   72.54%   73.00%   +0.45%     
==========================================
  Files         103      103              
  Lines        5150     5126      -24     
  Branches      407      407              
==========================================
+ Hits         3736     3742       +6     
+ Misses       1396     1366      -30     
  Partials       18       18              
Impacted Files Coverage Δ
src/application/App.tsx 0.00% <0.00%> (ø)
src/components/Spinner/index.tsx 0.00% <ø> (ø)
...es/textEditor/components/tipTapNodes/test-utils.ts 100.00% <ø> (+3.00%) ⬆️
src/io/filesystem.ts 25.43% <66.66%> (+7.98%) ⬆️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@danvk danvk marked this pull request as ready for review June 29, 2023 19:23
package.json Show resolved Hide resolved
src/components/Spinner/index.tsx Show resolved Hide resolved
sehyod
sehyod previously approved these changes Jun 30, 2023
Copy link
Collaborator

@sehyod sehyod left a comment

Choose a reason for hiding this comment

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

I didn't know about this tool, it's really nice. Thanks for setting it up!

@@ -31,24 +29,7 @@ async function getBaseDir() {
export async function getUploadsDir() {
return join(await getBaseDir(), UPLOADS_DIR);
}
export async function ensureProjectFileStructure() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit surprised we aren't using this anymore 🤔
@cguedes is this intentional?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sehyod no! I'm also a bit surprised. I think that this is a great gateway to have some (proper) initial/demo structure for RefStudio.

What do you think of calling this method from the settings to reset the editor data?

Copy link
Collaborator

@sehyod sehyod Jun 30, 2023

Choose a reason for hiding this comment

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

I think we have to call it on start, because we need to create the base directory for the app when the app starts for the first time: await createDir(baseDir, { recursive: true });
I'm not sure it would work as a reset function, because it wouldn't remove created and uploaded files

Copy link
Collaborator

@cguedes cguedes Jun 30, 2023

Choose a reason for hiding this comment

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

Sure, we would need to improve that for the "initial/demo/reset" feature (in a new issue).
For now I will push a commit to use it in the app startup.

EDIT: I'm also updating the file names in that function to be .refstudio instead of .tiptap.

Copy link
Collaborator

@cguedes cguedes left a comment

Choose a reason for hiding this comment

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

We should agree on what to do with these two functions.

src/components/Spinner/index.tsx Show resolved Hide resolved
@@ -31,24 +29,7 @@ async function getBaseDir() {
export async function getUploadsDir() {
return join(await getBaseDir(), UPLOADS_DIR);
}
export async function ensureProjectFileStructure() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sehyod no! I'm also a bit surprised. I think that this is a great gateway to have some (proper) initial/demo structure for RefStudio.

What do you think of calling this method from the settings to reset the editor data?

@cguedes cguedes merged commit f25cc9e into main Jun 30, 2023
10 of 11 checks passed
@cguedes cguedes deleted the knip branch June 30, 2023 09:33
@sehyod sehyod mentioned this pull request Jul 6, 2023
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