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 test: Reduce lodash depend code #3290

Merged
merged 1 commit into from Oct 16, 2018

Conversation

3 participants
@segayuu
Contributor

segayuu commented Oct 15, 2018

Thank you for creating a pull request to contribute to Hexo code! Before you open the request please review the following guidelines and tips to help it be more easily integrated:

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

@segayuu segayuu requested a review from hexojs/core Oct 15, 2018

@coveralls

This comment has been minimized.

coveralls commented Oct 15, 2018

Coverage Status

Coverage remained the same at 97.27% when pulling 2416b38 on segayuu:Fix-Test-reduce-lodash-depend-code into e6cac92 on hexojs:master.

const should = require('chai').should(); // eslint-disable-line
'use strict';
const should = require('chai').should();

This comment has been minimized.

@YoshinoriN

YoshinoriN Oct 15, 2018

Member

It seems not related about lodash.
I think it should be separated it into other pull-request.

This comment has been minimized.

@segayuu

segayuu Oct 16, 2018

Contributor

Yes, your review is correct.
In that part, we refactor the behavior with no change in the difference of only two lines of the file.
In other words, this refactoring is a greedy result, oh .

const should = require('chai').should(); // eslint-disable-line
'use strict';
const should = require('chai').should();

This comment has been minimized.

@YoshinoriN

YoshinoriN Oct 15, 2018

Member

Also this.

const should = require('chai').should(); // eslint-disable-line
'use strict';
require('chai').should();

This comment has been minimized.

@YoshinoriN

YoshinoriN Oct 15, 2018

Member

Also this.

Btw, does it work well ? Sorry, I'm not investigate chai.should behavior, I don't know this code is good or not.

This comment has been minimized.

@segayuu

segayuu Oct 16, 2018

Contributor

Even if we abandon the return value that calls should() like the sample code of https://www.chaijs.com/guide/styles/#using-should-in-es2015, in principle no problem.
By calling should(), the should property is added to Object.prototype via Object.defineProperty(). You can use should from almost any value after that.
However, since only null, undefined, or prototype null objects can not call Object.prototype from the prototype chain, you can not get should property.
The return value of should() is used to assert such as should.not.exist(undefined) for these values.

const _ = require('lodash');
'use strict';
require('chai').should();

This comment has been minimized.

@YoshinoriN

YoshinoriN Oct 15, 2018

Member

Also this.

const should = require('chai').should(); // eslint-disable-line
'use strict';
require('chai').should();

This comment has been minimized.

@YoshinoriN

YoshinoriN Oct 15, 2018

Member

Also this.

@YoshinoriN

LGTM!

@YoshinoriN YoshinoriN merged commit d558ac0 into hexojs:master Oct 16, 2018

3 checks passed

codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@segayuu segayuu deleted the segayuu:Fix-Test-reduce-lodash-depend-code branch Oct 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment