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

add component to display contributors #129

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

grmbyrn
Copy link
Collaborator

@grmbyrn grmbyrn commented Nov 27, 2024

  • in routes/(home)/+page.server.ts, fetch contributors data from GitHub API to use in Torrust repos.

    • Create a Contributor type for data coming from GitHub API and store it in '$lib/utils/types' to be imported
    • Create an array of repo names and store it in '$lib/constants/constants' to be imported and reactively mapped over together with a baseURL
    • Create a Personal Access Token, store it in .env and store it in const token = import.meta.env.VITE_GITHUB_TOKEN; to order to increase GitHub API rate limits.
    • Fetch contributor data from multiple GitHub repository URLs concurrently using Promise.all. It sends authenticated GET requests (with a token in the headers) to each URL and parses the JSON response for all the requests.
    • Flatten the array of contributor arrays into a single array of all contributors.
    • Remove duplicate contributors from the all contributors array.
    • Pass array into routes/(home)/+page.svelte, which passes it into component Contributors.svelte in order to loop over and display each contributor on the home page.
  • On each of the (pages) routes there wasn't responsiveness to make Table of Contents appear above the text on smaller screens and to the left on wider screens, so I added that to each.

Comment on lines 710 to 735
export const repoNames = [
'torrust-tracker',
'torrust-compose',
'torrust-website',
'torrust-index',
'torrust-index-gui',
'torrust-bencode-online',
'torrust-demo',
'bittorrent-infrastructure-project',
'bencode2json',
'torrust-installer',
'bittorrent-primitives',
'json2bencode',
'torrust-bencode2json',
'torrust-index-api-lib',
'torrust-index-types-lib',
'torrust-hash2torrent',
'torrust-index-gui-user-guide',
'containerizing-rust-apps-examples',
'torrust-serde-bencode-archive',
'torrust-index-archive',
'torrust-parse-torrent',
'torrents-importer-sample',
'torrust-documentation',
'torrust-docker'
];
Copy link
Member

Choose a reason for hiding this comment

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

Hi @grmbyrn I guess you can also get the list of repos from the API.

src/routes/(home)/+page.server.ts Outdated Show resolved Hide resolved
@josecelano
Copy link
Member

ACK 158a94e

Copy link
Member

@josecelano josecelano left a comment

Choose a reason for hiding this comment

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

Hi @grmbyrn, when you run it locally you get this error:

image

  ➜  Local:   http://localhost:5173/
  ➜  Network: http://192.168.1.37:5173/
  ➜  Network: http://192.168.1.33:5173/
  ➜  press h + enter to show help
Fetching data from API
Rate limit: 60, Remaining: 0 10:46 AM
Rate limit resets at: Mon Dec 02 2024 10:53:01 GMT+0000 (Western European Standard Time)
Failed to fetch from https://api.github.com/orgs/torrust/repos: rate limit exceeded
Error: Failed to fetch from https://api.github.com/orgs/torrust/repos
    at Module.fetchWithCache (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/src/lib/utils/cache.ts:40:9)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async load (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/src/routes/(home)/+page.server.ts:13:16)
    at async Module.load_server_data (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/page/load_data.js:61:17)
    at async eval (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/page/index.js:140:13)
Fetching data from API
Rate limit: 60, Remaining: 0 10:46 AM
Rate limit resets at: Mon Dec 02 2024 10:53:01 GMT+0000 (Western European Standard Time)
Failed to fetch from https://api.github.com/orgs/torrust/repos: rate limit exceeded
Error: Failed to fetch from https://api.github.com/orgs/torrust/repos
    at Module.fetchWithCache (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/src/lib/utils/cache.ts:40:9)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async load (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/src/routes/(home)/+page.server.ts:13:16)
    at async Module.load_server_data (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/page/load_data.js:61:17)
    at async Promise.all (index 2)
    at async Module.render_data (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/data/index.js:99:17)
    at async resolve (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/respond.js:436:17)
    at async Module.respond (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/respond.js:322:20)
    at async file:///home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/exports/vite/dev/index.js:524:22

I guess it's because I restarted the server and the data is not in the store anymore. I think we should not show the contributors list in that case.

I would also add the list to the community page.

Finally, I would remove console logs for production, especially when it's only for debugging.

TODO

  • Fix problem when get too many requests error but the store is empty. I think we could have a default value in this case with the list of contributor. We can update that default value from time to time.
  • Remove console logs
  • Add the list to the community page

@grmbyrn
Copy link
Collaborator Author

grmbyrn commented Dec 2, 2024

Hi @grmbyrn, when you run it locally you get this error:

image

  ➜  Local:   http://localhost:5173/
  ➜  Network: http://192.168.1.37:5173/
  ➜  Network: http://192.168.1.33:5173/
  ➜  press h + enter to show help
Fetching data from API
Rate limit: 60, Remaining: 0 10:46 AM
Rate limit resets at: Mon Dec 02 2024 10:53:01 GMT+0000 (Western European Standard Time)
Failed to fetch from https://api.github.com/orgs/torrust/repos: rate limit exceeded
Error: Failed to fetch from https://api.github.com/orgs/torrust/repos
    at Module.fetchWithCache (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/src/lib/utils/cache.ts:40:9)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async load (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/src/routes/(home)/+page.server.ts:13:16)
    at async Module.load_server_data (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/page/load_data.js:61:17)
    at async eval (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/page/index.js:140:13)
Fetching data from API
Rate limit: 60, Remaining: 0 10:46 AM
Rate limit resets at: Mon Dec 02 2024 10:53:01 GMT+0000 (Western European Standard Time)
Failed to fetch from https://api.github.com/orgs/torrust/repos: rate limit exceeded
Error: Failed to fetch from https://api.github.com/orgs/torrust/repos
    at Module.fetchWithCache (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/src/lib/utils/cache.ts:40:9)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async load (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/src/routes/(home)/+page.server.ts:13:16)
    at async Module.load_server_data (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/page/load_data.js:61:17)
    at async Promise.all (index 2)
    at async Module.render_data (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/data/index.js:99:17)
    at async resolve (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/respond.js:436:17)
    at async Module.respond (/home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/runtime/server/respond.js:322:20)
    at async file:///home/josecelano/Documents/git/committer/me/github/torrust/torrust-website/node_modules/@sveltejs/kit/src/exports/vite/dev/index.js:524:22

I guess it's because I restarted the server and the data is not in the store anymore. I think we should not show the contributors list in that case.

I would also add the list to the community page.

Finally, I would remove console logs for production, especially when it's only for debugging.

TODO

  • Fix problem when get too many requests error but the store is empty. I think we could have a default value in this case with the list of contributor. We can update that default value from time to time.
  • Remove console logs
  • Add the list to the community page

TODO

  • Fix problem when get too many requests error but the store is empty. I think we could have a default value in this case with the list of contributor. We can update that default value from time to time.
  • Remove console logs
  • Add the list to the community page

Hi @josecelano for the request error, I made an array of objects for the contributors and put it in constants. It will display the data coming from constants instead of the data coming from the GitHub API when the try catch block in +page.server.ts detects an error.

src/lib/utils/cache.ts Outdated Show resolved Hide resolved
src/lib/constants/constants.ts Outdated Show resolved Hide resolved
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.

2 participants