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.

Sign up for GitHub

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

Jump to bottom

refactor(theme/view): drop lodash.assignIn #3969

Merged
merged 1 commit into from Dec 16, 2019
Merged

refactor(theme/view): drop lodash.assignIn #3969

merged 1 commit into from Dec 16, 2019

Conversation

SukkaW
Copy link
Member

@SukkaW SukkaW commented Dec 15, 2019

What does it do?

The last part of #3753, drop lodash.assignIn in lib/theme/view.

Port from _.extend in underscore.js.

How to test

git clone -b drop-lodash-assignIn https://github.com/sukkaw/hexo.git
cd hexo
npm install
npm test

Screenshots

Pull request tasks

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

@SukkaW SukkaW requested a review from curbengh December 15, 2019 23:20
@coveralls
Copy link

coveralls commented Dec 15, 2019 •
edited

Coverage Status

Coverage increased (+0.009%) to 97.143% when pulling 2aa1bc3 on SukkaW:drop-lodash-assignIn into 457fcfc on hexojs:master.

@curbengh curbengh added this to To do in Replace lodash with native API via automation Dec 16, 2019
curbengh
curbengh previously approved these changes Dec 16, 2019
View reviewed changes
Copy link
Contributor

@curbengh curbengh left a comment

Choose a reason for hiding this comment

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

Remember to remove line 4.

Replace lodash with native API automation moved this from To do to In progress Dec 16, 2019
@SukkaW
Copy link
Member Author

SukkaW commented Dec 16, 2019 •
edited

Node.js Version Cold Process Time Memory
8 Current master branch 23.60s 592MB
8 This PR 29.50s 590.1MB
10 Current master branch 24.38s 591MB
10 This PR 20.52s 593MB
12 Current master branch 23.75s 568.6MB
12 This PR 25.75s 573.5MB
13 Current master branch 25.81s 580.6MB
13 This PR 26.27s 570.8MB

Benchmark shows no regression.

@SukkaW
Copy link
Member Author

SukkaW commented Dec 16, 2019

@curbengh Removed & rebased.

@SukkaW SukkaW added this to the 4.2.0 milestone Dec 16, 2019
@SukkaW SukkaW requested a review from curbengh December 16, 2019 01:50
@SukkaW SukkaW merged commit f37557a into hexojs:master Dec 16, 2019
Replace lodash with native API automation moved this from In progress to Done Dec 16, 2019
This was referenced Dec 16, 2019
Closed
Merged
@SukkaW
Copy link
Member Author

SukkaW commented Dec 16, 2019

I have opened a PR at You-Dont-Need-Lodash-Underscore to bring our solution back to the community.

This was referenced Dec 18, 2019
Merged
Merged
thom4parisot pushed a commit to thom4parisot/hexo that referenced this pull request Jan 17, 2020
@SukkaW
refactor(theme/view): drop lodash.assignIn (hexojs#3969)
b05a41c
@snyk-bot snyk-bot mentioned this pull request Mar 16, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@curbengh curbengh curbengh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Replace lodash with native API
  
Done
Milestone
4.2.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@SukkaW @coveralls @curbengh