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

Feature: Added Gitalk support. #2037

Closed
wants to merge 2 commits into from
Closed

Feature: Added Gitalk support. #2037

wants to merge 2 commits into from

Conversation

AlynxZhou
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes have been added (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs have been added / updated (for bug fixes / 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?

A similiar pull request has been created but due to some reasons didn't pass.

Issue Number(s): N/A

What is the new behavior?

Description about this pull, in several words...

screenshot from 2017-12-17 11-08-18
screenshot from 2017-12-17 11-08-28

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

How to use?

In NexT _config.yml:

...

Does this PR introduce a breaking change?

  • Yes.
  • No.

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Dec 17, 2017

@AlynxZhou as i see, this looks like on Github? But this is save comments like Gitment, right? If yes, i think need to add config on this right into Gitmint/Gitment for reverse compatibility.
And yeah, looks very good!

@AlynxZhou
Copy link
Contributor Author

AlynxZhou commented Dec 17, 2017 via email

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Dec 17, 2017

And in fact just css and js styles have different, right? So, i think about migrate this styles to Gitmint/Gitment and peoples can choice what style they want: standart Gitment style, or this, modernize style. That's what i'm talking about.

For now this have modern style but use standart imsun's Gitment gateway, and i don't appreciate it (see my comment about this here).

I understand what it's pretty beautifull Gitment patch, but what users will do if they GitHUB account's will be breaking down? Who will answer for this doings if thousand peoples will talk something like: i installed NexT theme and turn on cool comments system, but my Github accound was stolen then. This is just example, think about it.

We can to do hardfork of this styles, just a variant.

OR

We can try to ask author to adapt he's styles for our mint/ment system in NexT, and this will be best decision.

@AlynxZhou
Copy link
Contributor Author

OK, I am back to my dorm and have read a lot words about it because I know little, firstly I add gitalk just because original gitment is inactive for a long time, and don't know things about gitmint and client secret, finally I think I understand what you mean.
You mean that using client secret like gitalk/gitment is insecure and NexT uses a fork called gitmint which allows user to build a own proxy to hide client secret. So we need something like mint to work with gitalk, that's what you mean, right? However this seems beyond my ability, I may ask gitalk author to add, or try to fork it myself but not now because I need to learn how to make it and I need to pass my final term exam first. ;-)
Please tell me if I misunderstood your words.

@ivan-nginx
Copy link
Collaborator

That's right.

@ivan-nginx ivan-nginx mentioned this pull request Jan 19, 2018
15 tasks
@SharryXu
Copy link

Hi @ivan-nginx , will you merge this pull request?

@gavin-io
Copy link

ok

@ivan-nginx
Copy link
Collaborator

Already added in NexT 6 here: theme-next/hexo-theme-next#464


NexT is rebased into organization repo.
If you want new feature, please, create pull in NexT v6.x repo.
There is instructions on English or Chinese how to update from v5.1.x to v 6.x

You also may read this for details.

@ivan-nginx ivan-nginx closed this Dec 12, 2018
Repository owner locked as resolved and limited conversation to collaborators Dec 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants