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

Replace String#substring() with String#startsWith() or String#endsWith() #3124

Merged
merged 10 commits into from May 5, 2018

Conversation

6 participants
@segayuu
Contributor

segayuu commented Apr 18, 2018

And String#indexOf() with String#includes()

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

@segayuu segayuu requested a review from hexojs/core Apr 18, 2018

@coveralls

This comment has been minimized.

coveralls commented Apr 18, 2018

Coverage Status

Coverage increased (+0.06%) to 97.273% when pulling 654741f on segayuu:substring-to-startswith-or-endswith into 373b9c7 on hexojs:master.

h404bi and others added some commits Apr 20, 2018

Merge pull request #3129 from h404bi/feature/meta-generator
Misc: add meta generator injection filter
added command line option `output` to write db.json and _multiconfig.… (
#3122)

* added command line option `output` to write db.json and _multiconfig.yml to a path provided

* fixes for appveyor
updated config loader to accept absolute paths to config files (#3118)
* updated config loader to accept absolute paths to config files

* fixes for coveralls and codeclimate

* refactored isAbsolute() check
@segayuu

This comment has been minimized.

Contributor

segayuu commented Apr 30, 2018

Since I have not responded for more than a week since I requested a review, I will notify @hexojs/core member once more.
If there is no response after one week from this comment, we will merge.

@segayuu segayuu requested review from hexojs/core and removed request for hexojs/core Apr 30, 2018

@segayuu segayuu changed the title from string.prototype.substring() to string.prototype.startsWith() or string.prototype.endsWith() to String#substring() to String#startsWith() or String#endsWith() Apr 30, 2018

@segayuu segayuu changed the title from String#substring() to String#startsWith() or String#endsWith() to Replace String#substring() with String#startsWith() or String#endsWith() Apr 30, 2018

@ivan-nginx

This comment has been minimized.

ivan-nginx commented Apr 30, 2018

@segayuu > If there is no response after one week

Why week? Go about 1-2 month? It's normal period for awaiting something important.

Also, i see codeclimate issues, mb u can fix them?

@@ -0,0 +1,17 @@
'use strict';

This comment has been minimized.

@ivan-nginx

ivan-nginx Apr 30, 2018

Also, it seems new filter added here. But in pull readme there is nothing about this.

This comment has been minimized.

@segayuu

segayuu May 1, 2018

Contributor

That change is the result of #3129 being resolved by running git rebase.

@segayuu

This comment has been minimized.

Contributor

segayuu commented May 1, 2018

@ivan-nginx
I argue that this PR is refactoring enabled as a result of clarification of the supported version of node.js.
Current Hexo has many codes premised on node.js less than v6, and it tends to lack maintainability.
This PR standalone is not accompanied by a change in API behavior. (As a result of rebasing, we have commits that are not so.)

The error issued by codeclimate is complexity. Somebody needs to split the function to fix it.
The fix will be extensive as a modification in the file.

@JLHwung

JLHwung approved these changes May 5, 2018

bravo!

@JLHwung JLHwung merged commit 0b26940 into hexojs:master May 5, 2018

3 of 4 checks passed

codeclimate 2 issues to fix
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.06%) to 97.273%
Details

@segayuu segayuu deleted the segayuu:substring-to-startswith-or-endswith branch May 7, 2018

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