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

Get GitHub issues from dependency repo #3295

Merged
merged 1 commit into from
Mar 25, 2022

Conversation

JerryHue
Copy link
Contributor

@JerryHue JerryHue commented Mar 23, 2022

Issue This PR Addresses

Fixes #2827

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

This PR adds the whole GitHub issue thing to collect issues that are considered newcomer friendly.

A couple of details

This doesn't use Redis, at all. I tried to use Redis at first, but it seems that using it is somewhat redundant when the githubInformationRequestor is used. The reason I use this is to minimize the number of requests as possible when there are concurrent requests to the same resource. If we program this naively, when there are two or more requests really close to each other, all of those requests will ask essentially the same information, and thus we would be wasting requests.

I will file a couple of issues to address some stuff that have to be changed:

  • use Redis instead of a requestor, this would imply some rewriting of the whole service, probably.
  • use a GitHub Key for the service to use so that we have an increased rate limit on staging and production.

Steps to test the PR

  1. Pull the changes
  2. Go to the dependency-discovery folder.
  3. Start the service with pnpm dev.
  4. You should be able to request the service from your browser. For example, GET https://localhost:10500/github/@babel/core

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@JerryHue JerryHue added the area: dependency visualization Shows what dependencies are used in Telescope label Mar 23, 2022
@JerryHue JerryHue added this to the 2.9 Release milestone Mar 23, 2022
@JerryHue JerryHue requested review from humphd and DukeManh March 23, 2022 17:22
@JerryHue JerryHue self-assigned this Mar 23, 2022
@gitpod-io
Copy link

gitpod-io bot commented Mar 23, 2022

src/api/dependency-discovery/src/github.js Outdated Show resolved Hide resolved
src/api/dependency-discovery/src/github.js Show resolved Hide resolved
src/api/dependency-discovery/src/github.js Outdated Show resolved Hide resolved
src/api/dependency-discovery/src/github.js Outdated Show resolved Hide resolved
@TueeNguyen
Copy link
Contributor

Can you supply an example request? I tried /github/motdotla/dotenv but didn't work

@JerryHue
Copy link
Contributor Author

JerryHue commented Mar 24, 2022

Can you supply an example request? I tried /github/motdotla/dotenv but didn't work

I think you need to write it like /github/@motdotla/dotenv.

@AmasiaNalbandian
Copy link
Contributor

Hello! Please rebase, as the following commit 6046273 has reorganized our file structure!

humphd
humphd previously approved these changes Mar 25, 2022
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

I think this can go in, and we can do the rest in follow-ups (including tests).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dependency visualization Shows what dependencies are used in Telescope
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get and expose npm + GitHub data for dependency packages
6 participants