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

Added contributors endpoint for github #441

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

Conversation

dgmouris
Copy link
Collaborator

@dgmouris dgmouris commented Nov 1, 2024

What issue is this referencing?

#323

This PR essentially approaches the backend part of the contributors.

The only thing that we're going to need here is to put the Github API key as an environment variable for this to work.

Once this is merged (a little like the calendar) it'll be open season to create the page.

I really appreciate any reviews, and I hope this helps:)

Do these code changes work locally and have you tested that they fix the issue yourself?

  • yes!

Does the following command run without warnings or errors?

  • yes! npm run pr-checks

Have you taken a look at our contributing guidelines?

  • yes!

My node version matches the one suggested when running nvm use?

  • yes!

@dgmouris dgmouris requested a review from arashsheyda November 1, 2024 18:58
@arashsheyda
Copy link
Collaborator

arashsheyda commented Nov 1, 2024

also if we add .get or .post or ... to the file name, that would be the only supported method for that endpoint e.g. contributors.get.js

and if we used TS and defined the returned type, we automatically get them in the frontend of Nuxt

but do you think it's a good idea or that would be confusing for others?

@dgmouris
Copy link
Collaborator Author

dgmouris commented Nov 1, 2024

also if we add .get or .post or ... to the file name, that would be the only supported method for that endpoint e.g. contributors.get.js

I agree here that is a good idea. I'll whip this up shortly.

Let me take a bit of a gander at the TS piece.

@arashsheyda
Copy link
Collaborator

thanks!

@dgmouris
Copy link
Collaborator Author

dgmouris commented Nov 15, 2024

An important note about this pull request.

You'll need to include the GitHub API KEY as an environment variable this can be found under:

  • Organization Settings
    • Developer Settings
      • Personal Access Tokens
        • The permissions needed are as follows:
          • repo
          • repo
            • read:org

Just a note here we might have to refresh it somehow or make it not expire but I don't know if that's an option.

@dgmouris dgmouris force-pushed the add_maintainers_to_site_323 branch from d85d9a7 to 7d2dae5 Compare November 29, 2024 21:59
@MandyMeindersma
Copy link
Contributor

And with github API keys do I just have to like "donate" my personal key?

Was wondering if there was a group specific or repo specific key but I couldn't find anything

@dgmouris
Copy link
Collaborator Author

dgmouris commented Dec 6, 2024

And with github API keys do I just have to like "donate" my personal key?

Was wondering if there was a group specific or repo specific key but I couldn't find anything

I think that we can get an organizational key @MandyMeindersma, but I'm not sure. Let me take a look into this a bit deeper.

I'll update something shortly.

@dgmouris
Copy link
Collaborator Author

dgmouris commented Dec 9, 2024

@MandyMeindersma there isn't a specific way to add it to an organization. I think the only way would be a "personal token donation" or we can create an account that is specifically for devedmonton to create an API KEY.

Let me know what you think might be most suitable.

@MandyMeindersma
Copy link
Contributor

I think I am good with donation! I will try to get that configured before Friday!

@MandyMeindersma
Copy link
Contributor

image
image

Okay I think I set it up correctly?

@dgmouris
Copy link
Collaborator Author

@MandyMeindersma I think it's all good to go, I don't have access to netlify.

The deploy preview isn't working but I'm not sure if that's because it hasn't applied yet.

Is there a reason not to give it a similar scope as the other ids (I'm just not sure :) )

@MandyMeindersma
Copy link
Contributor

So netlify has an option to "treat as secret" which all API keys should be secret but we have never clicked it before. It is the same as another key though:

image

image

@MandyMeindersma
Copy link
Contributor

@MandyMeindersma
Copy link
Contributor

@MandyMeindersma
Copy link
Contributor

idk why it is not getting re connected to the pr
https://deploy-preview-441--dev-edmonton.netlify.app/

@MandyMeindersma
Copy link
Contributor

image

@MandyMeindersma
Copy link
Contributor

hehe this is where my javascript knowledge starts to die.

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.

3 participants