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(hexo_index): remove unused parameter #4153

Merged
merged 2 commits into from
Mar 6, 2020

Conversation

curbengh
Copy link
Contributor

What does it do?

immediate parameter is never called. Found by lgtm.

How to test

git clone -b unused-parameter https://github.com/curbengh/hexo.git
cd hexo
npm install
npm test

Screenshots

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.

@curbengh curbengh requested a review from SukkaW February 24, 2020 21:02
}, wait);
if (immediate && !timeout) func.apply(context, args);
if (!timeout) func.apply(context, args);
Copy link
Member

Choose a reason for hiding this comment

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

Since there's no immediate is given in actual practice, should we just remove this line as it is always false?

Copy link
Contributor Author

@curbengh curbengh Mar 6, 2020

Choose a reason for hiding this comment

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

my understanding is that, after wait time, timeout will become null, then the condition will become true and execute func.apply(). But func.apply() is already in L85 which will also be executed after wait, so looks like L84 is also useless.

@curbengh curbengh requested a review from SukkaW March 6, 2020 01:58
@stevenjoezhang
Copy link
Member

stevenjoezhang commented Mar 6, 2020

Relevant PR / commits:
#3845
f379187

https://lodash.com/docs/#debounce

@SukkaW SukkaW merged commit 0f3d00c into hexojs:master Mar 6, 2020
@curbengh curbengh deleted the unused-parameter branch March 10, 2020 22:07
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