learningequality / kolibri

Kolibri Learning Platform: the offline app for universal education
https://learningequality.org/kolibri/
MIT License
768 stars 644 forks source link

Fix/finish "Add Contributor to AUTHORS.md" action #11563

Open MisRob opened 9 months ago

MisRob commented 9 months ago

Observed behavior

"Add Contributor to AUTHORS.md" action doesn't work.

In the related pull request, https://github.com/learningequality/kolibri/pull/10902, we expected it to start working after merging it but it didn't.

I temporarily disabled this workflow on pull requests.

Expected behavior

The action works as expected and the disabled workflow on pull requests is re-enabled.

iskipu commented 9 months ago

hi @MisRob, i would like to try this can you assign this to me if possible ?

nikkuAg commented 9 months ago

Is this issue still open? I would like to work on this

MisRob commented 9 months ago

Hi @iskipu and @nikkuAg, thank you for volunteering! This issue is not open for contributions because someone with our org GitHub permissions will need to resolve it.

We currently have other contributing opportunities in three repositories. You can see the contributing guidelines including links to issues suitable for contribution for each repository here:

Kolibri: https://github.com/learningequality/kolibri/blob/release-v0.16.x/CONTRIBUTING.md Kolibri Design System: https://github.com/learningequality/kolibri-design-system/blob/main/CONTRIBUTING.md Studio: https://github.com/learningequality/studio/blob/unstable/CONTRIBUTING.md

thesujai commented 2 weeks ago

@rtibbles I had an idea while working on another project and wanted to see if you'd be open to it. How would you feel about adding a custom pre-commit hook that updates the AUTHORS.md file using a Python/JS script? The hook would be designed to skip this step if it's not running in a CI environment, as local git usernames might not match GitHub usernames (this was the case for me). Then the pre-commit-ci will handle updating the branch and we can eliminate the current add_contributor.md workflow.

Let me know your thoughts!

rtibbles commented 2 weeks ago

Hi @thesujai - that could work, definitely - although, I think I'd rather defer to github user accounts as the source of truth instead, as you can add any username locally, and could lead to unintentional entries - also, historically, we have found that the newest contributors who would need the pre-commit check to add their name to AUTHORS tend to be the ones that have not installed it.

I have also decided to only ever target these changes to the develop branch, so as to avoid merge conflicts from duelling AUTHORS entries - and I think this is the principle advantage of doing this in a github action, we can keep the AUTHORS file up to date, but without ever requiring it to be updated in a PR and generating an unnecessary merge conflict.

thesujai commented 2 weeks ago

Thank you for your feedback, and we can put an is_develop_branch in the script itself making the same workflow compatible for PR targeted to all branches, but updating the AUTHORS only when PR is made to develop.

rtibbles commented 2 weeks ago

That wasn't quite my intention - instead, the AUTHORS file should be updated when a PR is made to either a release branch or the develop branch, but we only ever update the file on the develop branch.