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

Remove unused function 'getName' #63

Merged
merged 1 commit into from
Feb 15, 2021
Merged

Remove unused function 'getName' #63

merged 1 commit into from
Feb 15, 2021

Conversation

atjn
Copy link
Contributor

@atjn atjn commented Feb 15, 2021

Hi! I wanted to help improve code coverage (#55), and the first thing I stumbled upon seems to be code that isn't used anymore. I believe this code can be removed.

getName checks to see if the optimize function has been passed an object instead of a string, in which case it tries to turn the object into a string. But it doesn't seem like it is possible to pass it an object. If I try to pass an object to minify, it will throw an error, and the documentation you wrote for optimize specifically calls for a string - no mention of an object.
In older versions of minify, it seems like some of these functions could be accessed much more directly, so I would guess that something was passing weird objects directly to optimize, and getName was built to accept those edge cases. That doesn't seem to be necessary anymore :)

I am also working on adding test coverage for the remaining parts of your code. Thank you for this awesome package!

This code serves no function and is therefore being removed
@coveralls
Copy link

Coverage Status

Coverage increased (+1.2%) to 96.923% when pulling 46e6b18 on atjn:rm-getname into 6528437 on coderaiser:master.

@coderaiser coderaiser merged commit 5341b55 into coderaiser:master Feb 15, 2021
@coderaiser
Copy link
Owner

Nice, thanks a lot :)

coderaiser pushed a commit that referenced this pull request Feb 15, 2021
This code serves no function and is therefore being removed
@coderaiser
Copy link
Owner

Landed in v7.0.1 🎉

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.

3 participants