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

feat: add gitalk #51

Merged
merged 4 commits into from Apr 5, 2018

Conversation

Projects
None yet
2 participants
@zhaozhiyuan1989
Contributor

zhaozhiyuan1989 commented Apr 3, 2018

fix #49

@olOwOlo

This comment has been minimized.

Show comment
Hide comment
@olOwOlo

olOwOlo Apr 3, 2018

Owner

谢谢你的PR~

  • 缩进麻烦对齐一下,两空格~
  • id 的话,我个人更倾向像之前的 gitment 一样用固定长度的 id: '{{ .Date }}'。或者,至少也应该是 id: decodeURI(location.pathname),默认的 location.href 里协议和域名都是多余的,很容易超过 label 的长度限制,他们的 README 里也显式指定了 id: location.pathname
  • 应该显式指定 title: '{{ .Title }}',对这个主题的标题 Title - Site Title 来说,后面的 - Site Title 有点多余
  • 是把 body 默认的 location.href + header.meta[description] 中的 description 有意忽略了吗?如果是,请使用 body: decodeURI(location.href),如果不是的话,用 body: decodeURI(location.href) + '\n\n' + '{{ .Summary }}'
Owner

olOwOlo commented Apr 3, 2018

谢谢你的PR~

  • 缩进麻烦对齐一下,两空格~
  • id 的话,我个人更倾向像之前的 gitment 一样用固定长度的 id: '{{ .Date }}'。或者,至少也应该是 id: decodeURI(location.pathname),默认的 location.href 里协议和域名都是多余的,很容易超过 label 的长度限制,他们的 README 里也显式指定了 id: location.pathname
  • 应该显式指定 title: '{{ .Title }}',对这个主题的标题 Title - Site Title 来说,后面的 - Site Title 有点多余
  • 是把 body 默认的 location.href + header.meta[description] 中的 description 有意忽略了吗?如果是,请使用 body: decodeURI(location.href),如果不是的话,用 body: decodeURI(location.href) + '\n\n' + '{{ .Summary }}'
@olOwOlo

This comment has been minimized.

Show comment
Hide comment
@olOwOlo

olOwOlo Apr 3, 2018

Owner

还有,用 jsdelivr 的 CDN,他们在国内有节点

Owner

olOwOlo commented Apr 3, 2018

还有,用 jsdelivr 的 CDN,他们在国内有节点

@zhaozhiyuan1989

This comment has been minimized.

Show comment
Hide comment
@zhaozhiyuan1989

zhaozhiyuan1989 Apr 4, 2018

Contributor

fix.

Contributor

zhaozhiyuan1989 commented Apr 4, 2018

fix.

@olOwOlo

This comment has been minimized.

Show comment
Hide comment
@olOwOlo

olOwOlo Apr 4, 2018

Owner

Thx,嘛,其实我的意思是只要不用 unpkg.com 就可以了,不过既然你加上了本地文件,也行吧 _(:з)∠)_

  • 这个两个文件在本地没有 sourceMap 文件(不用引入了),需要去掉最后一行的 sourceMappingURL
  • 这里在注释前加个空格对齐一下吧~
Owner

olOwOlo commented Apr 4, 2018

Thx,嘛,其实我的意思是只要不用 unpkg.com 就可以了,不过既然你加上了本地文件,也行吧 _(:з)∠)_

  • 这个两个文件在本地没有 sourceMap 文件(不用引入了),需要去掉最后一行的 sourceMappingURL
  • 这里在注释前加个空格对齐一下吧~
@zhaozhiyuan1989

This comment has been minimized.

Show comment
Hide comment
@zhaozhiyuan1989

zhaozhiyuan1989 Apr 5, 2018

Contributor

fixed.

Contributor

zhaozhiyuan1989 commented Apr 5, 2018

fixed.

@olOwOlo olOwOlo changed the title from add gitalk to feat: add gitalk Apr 5, 2018

@olOwOlo olOwOlo merged commit 5f8b79f into olOwOlo:master Apr 5, 2018

@zhaozhiyuan1989 zhaozhiyuan1989 deleted the zhaozhiyuan1989:zzy branch Apr 5, 2018

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