-
Notifications
You must be signed in to change notification settings - Fork 2
Devx2126 - Added the GitHub Discussions integration #238
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for the GitHub Discussions integration by updating backend dependencies, UI components, and configuration templates.
- Upgraded the search backend dependency to an alpha version required for the Discussions module.
- Added new dependencies and UI components for GitHub Discussions in both the backend and frontend.
- Extended configuration files with commented examples for GitHub Discussions.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/backend/src/index.ts | Updated search backend dependency and added the Discussions module. |
| packages/backend/package.json | Added dependency for the GitHub Discussions backend module. |
| packages/app/src/components/search/SearchResultCustomList.tsx | Added a new case and corresponding component for GitHub Discussions results. |
| packages/app/src/components/search/SearchPage.tsx | Updated search page to include GitHub Discussions option. |
| packages/app/package.json | Added dependency for the GitHub Discussions frontend plugin. |
| app-config*.yaml | Extended configuration samples with GitHub Discussions settings. |
Comments suppressed due to low confidence (1)
packages/app/src/components/search/SearchPage.tsx:79
- Ensure that the naming and versioning between the frontend plugin ('@backstage-community/plugin-github-discussions') and the backend module ('@backstage-community/plugin-search-backend-module-github-discussions') are clearly documented to avoid confusion in the integration.
{ value: 'github-discussions',
MonicaG
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this PR replaces PR #191 ?
I think the changes look reasonable, but I have some questions before we merge it in
| # discussionsBatchSize: 50 | ||
| # commentsBatchSize: 50 | ||
| # repliesBatchSize: 50 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to think about whether we want to set the configuration options that are commented out.
The documentation on the plugin indicates the queries are batched to so as not exceed GitHub's rate limits. When we are running in prod, with multiple instances, we could run into rate limits simply by the fact each container is going to be querying out to GitHub.
It's probably not something we need to be worried about now, but maybe something we need to think about in the future
| <GithubDiscussionsSearchResultListItem | ||
| key={document.location} | ||
| result={document} | ||
| highlight={highlight} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the screenshots in this PR, it looks like the body of the discussion is included in the results. Is that correct? Is there a way to configure this?
My concern is search is not protected by login. We could be displaying privileged information in the search results
sheaphillips
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good for this get merged in once @MonicaG comments are addressed.
Added the GitHub Discussions integration to DevHub
This pull request introduces support for GitHub Discussions integration into DevHub, enabling backend collating, and surfacing via search results.
I added two plugins:
@backstage-community/plugin-github-discussions@backstage-community/plugin-search-backend-module-github-discussionsChanges
GithubDiscussionsSearchResultListItemcomponent.Configuration Details
app-config.*files, this includes a new${GITHUB_DISCUSSIONS_URL}environment variable which is required for the backend to start.@backstage-community/plugin-search-backend-module-github-discussionsuses the GitHub Integrations settings, so the token/app requires read access to the Discussions url.Here's a screenshot of the search results page using the Rocket.Chat Discussions
I think we just need to settle on a Discussions repo then this is G2G.