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

[REQUEST] Code Cleanup: Remove Unused Imports and Variables #856

Open
Sweetdevil144 opened this issue May 5, 2024 · 6 comments
Open

[REQUEST] Code Cleanup: Remove Unused Imports and Variables #856

Sweetdevil144 opened this issue May 5, 2024 · 6 comments
Assignees
Labels
bug Something isn't working theme builder app Theme Builder application

Comments

@Sweetdevil144
Copy link

Code Cleanup Required

Suggestion/Concern

There are unused imports in some of the code files. For example, in one of the files, the hooks useRef and useLayoutEffect are imported from 'react' but are not used anywhere in the file code/src/ui/src/pages/DesignSystemPage.tsx. This is unnecessary and can lead to confusion.

Proposed Solution

Perform a thorough review of the codebase to identify any unused imports, variables, or other code elements. Once identified, these should be removed to keep the codebase clean and efficient. This will also help in improving the readability of the code and reduce the complexity. We can also add linter features to the same and implement linting before testing to keep track of such future issues.

Although I realise that MAYBE these will be utilised in near future, maintaining code quality is also an aspect we'll need to look over.

// For example, in the file `code/src/ui/src/pages/DesignSystemPage.tsx` we have:
import React, { useRef, useLayoutEffect, ReactNode } from 'react';

// But useRef and useLayoutEffect are not used anywhere in the file.
// So, the import statement should be cleaned up to:
import React, { ReactNode } from 'react';
@Sweetdevil144
Copy link
Author

Tagging @PaulaPaul , @aaronreed708 and @evangk6 for your views.
I'd love to hear your thoughts on the same.

@rahat2134
Copy link

@Sweetdevil144 can I go with the issue?

@Sweetdevil144
Copy link
Author

Hi @rahat2134 . I've already finished working on this issue and maintained end to end code quality locally. I'm waiting for the maintainer's review to create a PR for the same.

@aaronreed708
Copy link
Contributor

@Sweetdevil144 Thank you! Yes, we certainly have a lot of issues that lint would help with. If you have code ready to go, please submit a PR and I'll review it. Thank you for taking the time to open the issue and working on this!

@aaronreed708 aaronreed708 added bug Something isn't working theme builder app Theme Builder application labels May 8, 2024
@Sweetdevil144
Copy link
Author

Opening a PR soon as I get to my room :)

Sweetdevil144 added a commit to Sweetdevil144/a11y-theme-builder that referenced this issue May 9, 2024
@Sweetdevil144
Copy link
Author

Sweetdevil144 commented May 9, 2024

@aaronreed708 PR done and a custom .eslintrc.json was created. However, I'd like to remind that I haven't actually ran npm run lint:fix owing to the large amount of code cleanup. Approximately 35700 LOC changed when I tried doing it in a temp-branch (now deleted).

However, No issues were found to stop deployments after addition of linting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working theme builder app Theme Builder application
Projects
Status: In Progress
Development

No branches or pull requests

3 participants