Credits show contributors name if available

[HanLap]

trafficstars

Closes #456

This changes the update credits workflow step.

Contributors are listed as follows:

• if name exists: <name> (<login>)
• otherwise: <login>

[HanLap]

I just realized that my implementation is straining the API rate limit quite a bit. An API call is required for each Contributor. That currently means 93 calls.

If this project is subject to the same rate limit as personal users (1000/h), it is very likely to hit this limit.

Potential solutions would be:

• only build the detailed credit list for a main branch release.
• somehow cache the credits between workflows.

[captaincolonelfox]

@HanLap

API rate limit

Nice catch, I wasn't aware of that. We can pass headers: {"If-Modified-Since": date} to github.repos.listContributors request so it will return 304 Not Modified status code if there is no new contributors and no need to rebuild credits.txt

There is a way to cache/restore file (actions/cache) and a way to delete used cache (github.actions.deleteActionsCacheById)

It just require a further investigation how to properly use it

[idbrii]

I just read about this project from lazerwalker's Managing a game dev community with GitHub Actions post. She mentioned crediting in the post:

An important note on inclusivity: although both git commit history and the GitHub API contain contributor information, we very consciously chose to use the GitHub contributors API to use GitHub usernames rather than git commit names. If someone wants to change their name in the credits (for example, they’ve transitioned, and previous contributions were made under their deadname), updating a git-based credits screen would require rewriting git history, which can be disruptive for an open source project. Conversely, pulling real-time data from the GitHub API helped us ensure that we were always referring to people how they wanted to be.

I guess this PR still aligns with that view because it gets the name list every time a build is created? And caching would be time-limited so eventually all names get re-fetched?

Regardless, cool project!

[captaincolonelfox]

@idbrii Oh yeah, we need to take account of that aswell. Not sure if actions/cache have something built-in, but we can have week-number in cache key name. So even if number of contributors didn't changed, the credits will be rebuild anyway - so new names will be fetched

[HanLap]

Okay, here is what I implemented: Credits are cached for a single day. As long as there is a cached credits item for the given day the update credits step won't run at all. Furthermore, because actions/cache will only search for cache items of the current branch and its parent branch, I added a daily workflow that will build the credits cache for the main branch.

[captaincolonelfox]

@HanLap That's amazing how while I was thinking about the possible solution, you already implemented something, incredible job!

This solution looks good to me. Though, if there is a new contributor in that day (after credits get cached), the contributor will be added to a credits only after a new build after that day. Which actually may not happen for a week or even more.

Maybe we can daily run build-and-publish workflow aswell. Not sure if it's will trigger something unintended or broke version counting (or something). But I guess we should be ok

I will ask @lazerwalker to review this change as well

[hughesjs]

Is it worth noting that some people might not want their username being auto-translated into their real name even if it's available on their GitHub account?

A lot of people would rather build a reputation based on their pseudonym than their real name.

Maybe add a whitelist?

[captaincolonelfox]

@hughesjs This script will use both <name> (<login>). But maybe we need to consider highlighting login instead of name and switch them. Creating some login-only-filter.txt file is might be a good idea for those who only wants to have their login in the credits. But I would like to have it as a separate pull request after this one get merged, because it's already getting over complicated

[lazerwalker]

To take a step back, I believe this PR does three things:

1. Consistently shows both the GH username and display name in the credits
2. If I understand correctly, this changes it so credits are updated at most once a day. If I'm the second PR merged in a day, my name won't be added to the credits until the next day
3. Guarantees that the credits are updated even on days when no new contributions are merged

Under the current system / without this code, because GH inconsistently caches credit data, sometimes (but not always) someone whose code gets merged in might not see their name in the credits until a subsequent PR, which could be days. So (3) fixes an existing issue with credits not showing up, in addition to the new functionality added in (1).

As it stands, even if we say that (1) is unequivocally a positive change (which I'm not sure is the case), (2) feels like a large step back. I'm not sure the complexity we're adding is necessarily worth it here? Running a daily "do we have credits that need to be updated?" check makes a lot of sense to me to capture that edge case (and is something that could happen independent of the rest of this logic), but I'm not sure the questionable benefit of showing display names outweighs the credit lag being applied to a bunch of new contributions plus a bunch more workflow code to maintain.

Curious to hear others' thoughts! To be clear, this isn't me saying "booo, this is bad", I'm just curious how other folks view the cost/benefit analysis.

[captaincolonelfox]

@lazerwalker Yes, I'm totally agreed that additionally to what we already have in this pr, we need to update credits when there is a new contributor within a day.

1. Simple solution

Instead of daily credits check we can have it every hour

2. More technical solution

Sadly, there is no option to update cache for the same key, even if we overwrite the credits.txt file with the new data. But I find out that we can use restore-key in actions/cache as in this example:

key: credits-{date+time}
restore-keys: |
   credits-

So first of all it will look for credits cached for the key credits-{date+time}. And if no cache found, restore it from key credits- with the most recent creation date (credits-{date+time} from the same day or credits-{date} from daily build). And then it will save this file with new key credits-{date+time}.

My explanation may be bad, so here is screenshot how it works

But when cache restored from restore-keys, then cache-hit always will be false - which is doesn't matter in our case, because we also want to change check for update-credits to either:

• pass headers: {"If-Modified-Since": date} to github.repos.listContributors request so it will return 304 Not Modified status code if there is no new contributors and no need to overwrite credits.txt and we can leave it from cache
• or check if pull request was opened by first-time contributor ("author_association": "NONE") and rebuild credits only on this occasion

[captaincolonelfox]

@lazerwalker @HanLap What do you think about this solution? It would rebuild credits and cache them everyday (and push new build if credits are different from previous day), as well as rebuilding them on new contributor within a day and “update” cache - so every other build still using latest cached credits

@HanLap If it’s okay with you, I can implement that change and push commit to your branch

[HanLap]

I am sorry for not responding until now. ~_~

@captaincolonelfox It would be nice if you could implement the change.

[captaincolonelfox]

@HanLap I'm sorry, I tried to implement that, but immediately stuck

Didn't test those enough:

• {"If-Modified-Since": date} includes change in number of commits for any contributor - so it basically was triggered on any commit, not only on new contributor
• "author_association": "NONE" in context was only available on pull_request events, but not on main branch push event. And while we can rebuild credits on pull requests from new contributors, that cache will not be used in main branch later on

We need somehow check if push was done by first-time contributor, but I don't have anything particular in mind

[stale[bot]]

This PR hasn't been updated in a few weeks. Since this project can move quickly, it probably has a bunch of merge conflicts and can't cleanly be merged in! If this PR doesn't get updated within a week, it will be automatically closed. If that happens, feel free to open a new PR again with the same changes. Thanks!