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

Add comment_count option #559

Merged
merged 9 commits into from
Jan 5, 2019
Merged

Add comment_count option #559

merged 9 commits into from
Jan 5, 2019

Conversation

stevenjoezhang
Copy link
Contributor

@stevenjoezhang stevenjoezhang commented Jan 1, 2019 •
edited by ivan-nginx
Loading

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes was maked (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs in NexT website have been added / updated (for new features).

PR Type

What kind of change does this PR introduce?

  • Bugfix.
  • Feature.
  • Code style update (formatting, local variables).
  • Refactoring (no functional changes, no api changes).
  • Build related changes.
  • CI related changes.
  • Documentation content changes.
  • Other... Please describe:

What is the current behavior?

Issue Number(s): #271

What is the new behavior?

Description about this pull, in several words...

  • Screens with this changes: N/A
  • Link to demo site with this changes: N/A

How to use?

In NexT _config.yml:

valine:
...
  comment_count: true # if false, comment count will only be displayed in post page, not in home page

Does this PR introduce a breaking change?

  • Yes.
  • No.

@ivan-nginx ivan-nginx changed the title Fix #271 comment counter Fix #271 comment counter & Using Tamplate Literals Jan 2, 2019
@ivan-nginx ivan-nginx changed the title Fix #271 comment counter & Using Tamplate Literals Fix #271 comment counter & Using Template literals Jan 2, 2019
@ivan-nginx ivan-nginx changed the title Fix #271 comment counter & Using Template literals Comment counter & Using Template literals Jan 2, 2019
@stevenjoezhang
Copy link
Contributor Author

I have removed all Template literals in this PR, can it be merged now?

@ivan-nginx
Copy link
Member

Why you do this?) Literals will be used in NexT v7 (soon). Take a roll back, we will use literals by default in this year, it's good and conveniently.

Don't be so quickly. I said before: «Wait, i will create version branches, need to manage something with it».

@stevenjoezhang
Copy link
Contributor Author

This PR solved problem 1 in #271
And literals could be added to #557
Things are getting complicated ... I think 😂

@ivan-nginx
Copy link
Member

ivan-nginx commented Jan 3, 2019 •
edited
Loading

And literals could be added to #557

Ok, add it in #557 PR.
Also, try to separate feaures / bugfixes / optimizations in future. Just create separated local branch and you can move commits from any branch here. With Cherry-pick, for example. And try to use GitKraken.

@stevenjoezhang stevenjoezhang changed the title Comment counter & Using Template literals Comment counter Jan 3, 2019
@stevenjoezhang
Copy link
Contributor Author

@ivan-nginx Thanks for your advice, I have to admit I'm not so good at using git.
Besides, The current layout/_third-party/analytics/lean-analytics.swig(https://github.com/theme-next/hexo-theme-next/blob/master/layout/_third-party/analytics/lean-analytics.swig) is already using literals, and it was added in 3a921ce#diff-d4f96cf2cea92b9d7c8d95bb034e2eb2, but we haven't receive any error reports in the past several months. So, is it really important to keep supporting IE? (https://caniuse.com/#search=Template%20Literals) Maintaining two mainline versions would be challenging.

@ivan-nginx
Copy link
Member

ivan-nginx commented Jan 3, 2019 •
edited
Loading

IE will support only old v6.x, that's why need to separate versions. This is smooth rebase.
Template literals need to use in new v7.x. It's at least bring global refactoring in engine, so, major version must be changed.

Maintaining two mainline versions would be challenging

For now i'm write issue about it and as i see, it's didn't hard to maintenance. But need to make choice: what we will include and what not.

For now do what you do, create pulls into master (it's will be v7) and don't worry about it.

@ivan-nginx ivan-nginx added this to the v7.0.0 milestone Jan 5, 2019
@ivan-nginx ivan-nginx merged commit 23647f3 into theme-next:master Jan 5, 2019
@stevenjoezhang stevenjoezhang changed the title Comment counter Fix #271: Add comment_count option Jan 5, 2019
@ivan-nginx ivan-nginx removed the Support label Jan 7, 2019
@ivan-nginx ivan-nginx changed the title Fix #271: Add comment_count option Added comment_count option. Mar 16, 2019
@ivan-nginx ivan-nginx changed the title Added comment_count option. Added comment_count option Mar 16, 2019
@stevenjoezhang stevenjoezhang changed the title Added comment_count option Add comment_count option Sep 15, 2019
tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019
@stevenjoezhang @ivan-nginx
LeanCloud comment counter now loaded faster (theme-next#559)
48f1e67 
* Fix theme-next#271 comment counter

* Update _config.yml

* Update lean-analytics.swig

* Update lean-analytics.swig

* Update post.swig

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

@ivan-nginx ivan-nginx ivan-nginx approved these changes

Assignees
No one assigned
Labels
⭐ Feature
Projects
None yet
Milestone
v7.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@stevenjoezhang @ivan-nginx