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/giscus] add giscus support #536

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sgamerw
Copy link

@sgamerw sgamerw commented Aug 28, 2023

Description

add giscus support, not add config to site.config.ts, maybe too simple and crude, but it works.

Notion Test Page ID

I test it locally, and comment success to giscus repo.
https://github.com/orgs/giscus/discussions/1160

@vercel
Copy link

vercel bot commented Aug 28, 2023

@sgamerw is attempting to deploy a commit to the Saasify Team on Vercel.

A member of the Team first needs to authorize it.

@socket-security
Copy link

New and updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
@giscus/react 2.3.0 None +7 2.95 MB laymonage
react-notion-x 6.15.6...6.16.0 None +2/-2 2.24 MB fisch0920
notion-types 6.15.6...6.16.0 None +0/-0 77.4 kB fisch0920
notion-utils 6.15.6...6.16.0 None +1/-1 195 kB fisch0920
notion-client 6.15.6...6.16.0 None +2/-2 280 kB fisch0920

Comment on lines 15 to 18
repo="giscus/giscus"
repoId="MDEwOlJlcG9zaXRvcnkzNTE5NTgwNTM="
category="General"
categoryId="MDE4OkRpc2N1c3Npb25DYXRlZ29yeTMyNzk2NTc1"
Copy link

@laymonage laymonage Aug 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR, but please don't use these as the default, otherwise people who haven't configured the values with their own repo will spam the giscus repo with their discussions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, maybe just keep these configuration empty, then tell users how to set these in docs/wiki/readme?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't add configs to site.config.ts, there are too many values, so what's your suggestion?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laymonage already remove the giscus repo information, made these keys into config for giscus.

@transitive-bullshit
Copy link
Owner

This looks like a really great starting point for Giscus support, which I'm a fan of. I don't think it should be enabled by default or, as you mentioned, it should be optional and configured via the config file.

@sgamerw
Copy link
Author

sgamerw commented Dec 22, 2023

Thanks for reply, I'll add config for Giscus ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants