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

fix(box): ignore node_modules/ & .git/ in themes #3918

Merged
merged 3 commits into from Dec 12, 2019

Conversation

@curbengh
Copy link
Contributor

curbengh commented Dec 9, 2019

What does it do?

Continuation of #3797

Supersedes and closes hexojs/hexo-starter#30 cc @dailyrandomphoto

Related: #3885 hexojs/hexo-server#45 hexojs/hexo-server#43

How to test

cc @Shen-Yu

package.json
-  "hexo": "^4.1.0",
+  "hexo": "curbengh/hexo#ignore-theme-node",
  1. rm -f package-lock.json && rm -rf node_modules/
  2. npm install
  3. hexo clean && hexo server

Maintainers only:

git clone -b ignore-theme-node https://github.com/curbengh/hexo.git
cd hexo
npm install
npm test

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.
@curbengh curbengh changed the title fix(box): always ignore node_modules folder of themes fix(box): always ignore node_modules/ in themes Dec 9, 2019
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 9, 2019

Coverage Status

Coverage increased (+0.006%) to 97.137% when pulling a5c42d0 on curbengh:ignore-theme-node into c19096b on hexojs:master.

lib/box/index.js Show resolved Hide resolved
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Dec 9, 2019

As I couldn't reproduce the original issue, I will mark this ready for review after affected users have confirmed this PR does fix the issue.

@dailyrandomphoto

This comment has been minimized.

Copy link
Member

dailyrandomphoto commented Dec 10, 2019

As I couldn't reproduce the original issue, I will mark this ready for review after affected users have confirmed this PR does fix the issue.

It works for me.

@curbengh curbengh marked this pull request as ready for review Dec 10, 2019
@curbengh curbengh changed the title fix(box): always ignore node_modules/ in themes fix(box): ignore node_modules/ & .git/ in themes Dec 10, 2019
@curbengh curbengh requested a review from seaoak Dec 10, 2019
@curbengh curbengh added this to the 4.2.1 milestone Dec 10, 2019
@curbengh curbengh force-pushed the curbengh:ignore-theme-node branch from 2b087c1 to 1f4afac Dec 10, 2019
lib/box/index.js Outdated Show resolved Hide resolved
lib/hexo/default_config.js Outdated Show resolved Hide resolved
@curbengh curbengh requested a review from SukkaW Dec 11, 2019
@SukkaW
SukkaW approved these changes Dec 11, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Dec 11, 2019

The benchmark finished a lot quicker than usual.

Iterating through merged PRs, it starts from #3926.

We need to test the master branch in a test blog.

@SukkaW

This comment has been minimized.

Copy link
Member

SukkaW commented Dec 11, 2019

@curbengh I am already working on it.


TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at Hexo.postPermalinkFilter (/mnt/d/Project/Sukka/hexo-theme-unit-test/node_modules/hexo/lib/plugins/filter/post_permalink.js:46:24)
    at Filter.execSync (/mnt/d/Project/Sukka/hexo-theme-unit-test/node_modules/hexo/lib/extend/filter.js:75:28)
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Dec 11, 2019

It's from #3926, need null-check on config.permalink_defaults.

@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Dec 11, 2019

Since you're working on it, feel free to submit fix.

@curbengh curbengh merged commit 1336801 into hexojs:master Dec 12, 2019
3 of 4 checks passed
3 of 4 checks passed
codeclimate 2 issues to fix
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.006%) to 97.137%
Details
@curbengh curbengh deleted the curbengh:ignore-theme-node branch Dec 12, 2019
@SukkaW SukkaW mentioned this pull request Dec 12, 2019
1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.