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

Main page tweak - builders github activities #29

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

Conversation

indralukmana
Copy link
Contributor

@indralukmana indralukmana commented Aug 27, 2024

Description

This PR implements a Proof of Concept (POC) for ideas discussed in Issue #7.

Key Features

  1. Updated main page content to display builders and their "activity scores"
  2. Activity scores based on GitHub interactions within this repository:
    • Pull Requests
    • Issue comments
    • Issue comment reactions

Implementation Details

  • Utilizes GitHub's GraphQL API with octokit
  • Requires a GitHub access token (see Setup section)
  • Scoring system uses weighted values for different activities (currently arbitrary)
  • GitHub usernames retrieved using a temporary method (first committer or builder page creator)

Considerations

  1. API Rate Limits: Current implementation should work for low traffic, public information requests. I'm not exactly sure the exact limits, but based on my readings for small project with read only data should be fine.
  2. Scale: Current method should be suitable for a small number of builders (max 12 as per this comment)

Setup

  1. Create token at Create Token Link
  2. Update your .env file with a GitHub access token:
    NEXT_PUBLIC_GITHUB_TOKEN=your_token_here
    
  3. Refer to the .env.example file for the new entry

NOTE: I included test access token in octokit.ts file, this might or might not work in the future as this is a limited access token for test account with expiry

Improvements

These are some improvements that might be worth considering, however some of these might be out of scope for this project.

  • Create a static mapping of GitHub usernames to builder addresses
  • Implement a caching system or separate backend for data retrieval
  • Refine the scoring system weights
  • Explore more efficient methods for retrieving and storing GitHub usernames
  • Investigate and implement proper caching or backend solution for API requests
  • Conduct thorough testing with various numbers of builders to ensure scalability

Screenshots

💻 Desktop

FK5gvT7lwh

📱Mobile chrome_iKm4E0x5DH chrome_qdo8vrFcqG

Additional Information

Related Issues

#7

Your ENS/address: 0x80Ad2861Ab5D4EeA61330A4bd7d6969357C463C3

Other

GitHub GraphQL explorer https://docs.github.com/en/graphql/overview/explorer

Copy link

vercel bot commented Aug 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
batch8-buidlguidl-com-nextjs ❌ Failed (Inspect) Aug 30, 2024 7:35pm

@indralukmana
Copy link
Contributor Author

indralukmana commented Aug 27, 2024

I think Vercel don't want to play with hardcoded github access token, the deployment kept failing if using the hadcoded access token. I used the Vercel's environment variables to setup the NEXT_PUBLIC_GITHUB_TOKEN value and the deployment success on my deployment.

image

Copy link
Contributor Author

@indralukmana indralukmana left a comment

Choose a reason for hiding this comment

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

Thanks for the review @MexicanAce 👏 will do the adjustments

Copy link

vercel bot commented Aug 30, 2024

@indralukmana is attempting to deploy a commit to the BuidlGuidl Team on Vercel.

A member of the Team first needs to authorize it.

@derrekcoleman
Copy link
Member

I authorized the fork deployment to Vercel - hopefully that fixes the .env issue and lets us preview the page without having to set up locally

Copy link
Contributor

@RafaelCaso RafaelCaso left a comment

Choose a reason for hiding this comment

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

Hey Indra, everything looks good to me but I got a lot of TypeScript complaints in dataFetches.ts. In getBuildersPRData and getBuildersIssueActivities the types 'pr', 'issue', 'comment', 'reaction' and 'author' all have implicit type of 'any' so TS doesn't like it. Everything compiled fine for me and the page looks excellent so if it was my repo I wouldn't worry about it but if we want to be thorough maybe add some types for these.

type PullRequest = {
  author: {
    login: string;
  };
  state: "OPEN" | "CLOSED" | "MERGED";
}; 

etc.

@indralukmana
Copy link
Contributor Author

hey @RafaelCaso thank you for reviewing the PR 👏. I'm not sure if I'm getting the same issue. The types are already done in packages\nextjs\components\github-activities\githubQueriesTypes.ts and used as type parameter in octokit library. The types then inferred from the library usages.

image

Would you mind to check these?

  • run yarn next:check-types, to check if the types properly registered in your local
  • are you using vscode? can you try to do ctrl+shift+p or cmd+shift+p then reload window? sometimes TypeScript checking is lagging on newly fetched git / or branch and need to be refreshed.

@RafaelCaso
Copy link
Contributor

Yeah I checked those before. Like you said, the file githubQueriesTypes has the necessary types. I suspected it was just on my end. Other than that everything looks good to me, thanks for the very thorough documentation and responses

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.

4 participants