letsgamedev / Suffragium

A game developed in a democratic cycle.
GNU Affero General Public License v3.0
51 stars 21 forks source link

Tool Browser - Pull Request Overview #59

Closed Numenter closed 2 years ago

Numenter commented 2 years ago

Description

A browser site (local) to show an overview of the PRs merge condition states. For quick an easy access.

Images

image

image

Type of change

b7g commented 2 years ago

This looks great already. One thing to keep in mind is, that there is a rate limit of 60 requests to the API per hour, when not authenticated. That caps this application at 29 pull requests.

Numenter commented 2 years ago

Add an optional url parameter for personal access tokens? https://github.com/settings/tokens

Suffragium/tools/pr_merge_state/pr_merge_state.html?<token>

So everyone how needs it can save it as an bookmark? Is there a better option? load a local file?

b7g commented 2 years ago

Well, all I know is that (in some situations – when the rate limit would not suffice) we have to pass a PAT in some way, and that that should be safe.

I would build it like this:

Flow

  1. make the first request to the API, to see how many PRs there are
  2. if that first request is denied, the rate limit is reached, continue with Authentication
  3. if the remaining rate limit would suffice to fulfill the task, do it — else continue with Authentication

Authentication

  1. display a message that explains why authentication is needed
  2. let the user input their PAT to an <input> field
  3. authenticate
  4. continue in the Flow
Numenter commented 2 years ago

I put the token dialog in front. There is some client or server side caching , which makes it that, you have to wait a bit for the token to register.

BjoernAkAManf commented 2 years ago

We could just generate an overview every hour or on events though. No reason to make it client side.

IMO something like github pages (similar PR in that regard https://github.com/letsgamedev/Suffragium/pull/20 ). Cloudflare workers would also work as well.

Numenter commented 2 years ago

I don't know enough about GHPages and GHActions yet. So, local webpage it is. :P

b7g commented 2 years ago

Not quite sure if it had a reason why you haven’t added the automatic fetch without authentication, as it was just one line, but here you go: https://github.com/Numenter/Suffragium/pull/5

Numenter commented 2 years ago

One more fetch request is needed to get the approved reviews. Is it worth it? api review link

BjoernAkAManf commented 2 years ago

I don't know enough about GHPages and GHActions yet. So, local webpage it is. :P

Literally just needs a command to fetch stuff (what is done right now anyway), a way to supply the GH Token using an env var and writing the corresponding output to a text file. Then you could replace the calls to the github API with relative paths and it's done.

A static site generator might make it easier, but in theory a simple nodejs script would suffice.

Numenter commented 2 years ago

Yeah, it can only be used 3 times per hour (by 10 PRs) or 1 time with 29 PRs. For those who need more, can use a token. Is there a better solution? Yes, you mention a really good one. But this one does the job and is a massive upgrade to the current solution (manual filtering). At some point, someone, maybe you, maybe I or someone else will make it better.

b7g commented 2 years ago

Well, either @BjoernAkAManf or someone else is working on static site generation right now, or this should be accepted. As this is a big improvement from going into every PR manually to view the votes and manual filtering for inactive PRs.

RedstoneMedia commented 2 years ago

Wouldn't hosting this site on GitHub pages also work ? (Then it would need a separate branch though) Currently, it doesn't need any server code so hosting it with GitHub pages is trivial.

Numenter commented 2 years ago

We can select main, so we don't need an other branch. image Is there a option to use an other folder as root or /docs?