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

refactor: Delete unused functions fileReadArray and makeHtmlForRuby #1613

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

bryanjenningz
Copy link
Contributor

This pull request removes 2 unused functions in the data.ts file: fileReadArray and makeHtmlForRuby.

Feel free to close this pull request if we're planning on using these functions in the future.

If you use code search, you can see they're unused - so they can be safely removed:

Copy link
Owner

@melink14 melink14 left a comment

Choose a reason for hiding this comment

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

Thanks!

My opinion is that dead or unused code should never be left in. We can look at the history if we think we want it again.

If these are the last ones we may be able to add a compile or lint check to block future ones.

@mergify mergify bot merged commit deeaf9e into melink14:main Jun 12, 2023
@bryanjenningz bryanjenningz deleted the delete-unused-code branch June 12, 2023 04:03
@bryanjenningz
Copy link
Contributor Author

It's worth mentioning that TypeScript was unable to detect that these functions were unused because they were public functions. If we set these functions to private functions, then TypeScript gives an error saying that they're unused.

@melink14
Copy link
Owner

That makes sense; as a Chrome Extension, we know about all possible usages but Typescript doesn't have that context. (Also we set isolatedModules in tsconfig which means it only compiles one file at a time.)

This is probably a niche problem, mostly exacerbated by the non idiomatic typescript that resulted from migrating it all at once form legacy code. (That said, in a world where we break up the files into smaller modules, it would be nice to have this capability)

Thanks again!

@melink14
Copy link
Owner

🎉 This PR is included in version 2.5.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants