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: drop lodash for new-post-path filter #3813

Merged
merged 2 commits into from Nov 2, 2019

Conversation

@SukkaW
Copy link
Member

SukkaW commented Oct 27, 2019

What does it do?

This PR is part of #3753

How to test

git clone -b drop-lodash-for-new-post-path 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 Oct 27, 2019
@SukkaW SukkaW added this to To do in Replace lodash with native API via automation Oct 28, 2019
@SukkaW SukkaW requested a review from segayuu Oct 29, 2019
Replace lodash with native API automation moved this from To do to In progress Oct 29, 2019
Copy link
Member

YoshinoriN left a comment

Sorry, it's wrong. Please do not merge yet.

if permalink_defaults is null exctption occure.

TypeError: Cannot convert undefined or null to object
@YoshinoriN YoshinoriN dismissed their stale review Oct 29, 2019

I accidentally approved it.

Copy link
Member

YoshinoriN left a comment

I think we should set the default value at new_post_path.js#L26.

const permalinkDefaults = config.permalink_defaults || '';
@SukkaW

This comment has been minimized.

Copy link
Member Author

SukkaW commented Oct 29, 2019

@YoshinoriN

Maybe Object.assign({}, permalinkDefaults, filenameData) should be used? Then null will be ignored.

@YoshinoriN

This comment has been minimized.

Copy link
Member

YoshinoriN commented Oct 29, 2019

Maybe Object.assign({}, permalinkDefaults, filenameData) should be used?

I think it's better than set default value :)

@SukkaW SukkaW requested a review from YoshinoriN Oct 29, 2019
@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 29, 2019

Coverage Status

Coverage decreased (-0.1%) to 97.169% when pulling 330d88b on SukkaW:drop-lodash-for-new-post-path into c48f6a8 on hexojs:master.

1 similar comment
@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 29, 2019

Coverage Status

Coverage decreased (-0.1%) to 97.169% when pulling 330d88b on SukkaW:drop-lodash-for-new-post-path into c48f6a8 on hexojs:master.

@SukkaW SukkaW merged commit 0e59184 into hexojs:master Nov 2, 2019
3 of 4 checks passed
3 of 4 checks passed
coverage/coveralls Coverage decreased (-0.1%) to 97.169%
Details
Travis CI - Pull Request Build Passed
Details
codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Replace lodash with native API automation moved this from In progress to Done Nov 2, 2019
oncletom added a commit to oncletom/hexo that referenced this pull request Jan 17, 2020
* refactor: drop lodash for new-post-path filter
* fix: handle null case for new-post-path filter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.