Add a reminder when contributor is new to ping all contributor bot #2484

KevinEyo1 · 2024-03-29T18:33:49Z

What is the purpose of this pull request?

Overview of changes:
Fixes #2457
Added workflow to check on merge whether pr author is new to the repo, and ping a reminder to add them to contributors list

Anything you'd like to highlight/discuss:

Testing instructions:
test repo

Fork this repo and make any change and push it.
Create a PR
I can merge it and new contributor message pops up
Repeat steps 1 and 2
I can merge it and no more new contributor message pops up

Proposed commit message: (wrap lines at 72 characters)
GitHub Actions: ping new contributor

It is easy to forget to add new contributors.

Adding a workflow to automate reminders to add will help.

Let's on merge, check whether PR author has a prior merged PR,
pinging them if not since they are a new contributor.

This approach allows the user to remember to ask to be added to
the contributor list, if they want to.

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

Major (when you make incompatible API changes)
Minor (when you add functionality in a backward compatible manner)
Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

what was the old feature that was made obsolete

any replacement feature (if any), and

how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

codecov · 2024-03-29T18:39:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.00%. Comparing base (362f761) to head (7c00b6e).

❗ Current head 7c00b6e differs from pull request most recent head 03bde6f. Consider uploading reports for the commit 03bde6f to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2484   +/-   ##
=======================================
  Coverage   51.00%   51.00%           
=======================================
  Files         124      124           
  Lines        5384     5384           
  Branches     1162     1162           
=======================================
  Hits         2746     2746           
  Misses       2348     2348           
  Partials      290      290

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

LamJiuFong · 2024-03-30T04:21:09Z

.github/workflows/new-contributor.yml

+            const opts = github.rest.issues.listForRepo.endpoint.merge({
+              ...context.issue,
+              creator,
+              state: 'all'
+            })
+            const issues = await github.paginate(opts)
+
+            for (const issue of issues) {
+              if (issue.number === context.issue.number) {
+                continue
+              }
+
+              if (issue.pull_request) {
+                return // Creator is already a contributor.
+              }
+            }


This part lgtm but I am thinking of an alternative

Instead of checking all prs and see if there is any previous prs by the pr creator, I am thinking if we could utilise Github's built-in function to get the current list of contributors and check if the person is inside

https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#list-repository-contributors

Not sure if this approach would speed up the process since we do not have to go through every closed pr

Hi @LamJiuFong, thanks for the suggestion, I think it will be more efficient.

Thanks for the changes @KevinEyo1 ! LGTM

Hi @LamJiuFong after testing, I realised that since we are running the job on merge, the author is already part of the list by the time the action is run, so it fails to detect the author as a new contributor. I will revert to the original implementation, and I will try to figure out if there can be performance enhancements.

Oh I see, sorry for missing that out

What about checking if the author has only one pr merged (which is the newly merged one), would that be possible?

@LamJiuFong I am trying to think of a more efficient solution as well but I think to check if the author has only one PR merged we would have to fetch all the PRs anyway (which is what is done in the current implementation) leading to a similar time taken

Spoke to Kevin about this previously and he did this check: Test run to check time which wasn't very long. So I don't think there is a need to overoptimize in this case.

…markbind into 2457-ping-new-contrib

yucheng11122017 · 2024-04-01T14:14:30Z

Hi @KevinEyo1 thanks for the work! Could you also include some test workflows for us to check if its ok?

KevinEyo1 · 2024-04-02T04:13:00Z

Hi @yucheng11122017 I'm still working on the testing and fixing bugs, I will add the test workflows when I set this PR to ready for review.

yucheng11122017

LGTM!

…markbind into 2457-ping-new-contrib

KevinEyo1 · 2024-04-05T05:28:21Z

Test run to check time

This reverts commit 936dadc.

kaixin-hc

No comments on implementation, which I think looks good now!

However, I don't love the look of the commit message. Is there a reason why we're pinging the contributor and asking them to ask to be recognised? It feels less friendly. I'm OK with it if we also amend the readme/expectations as well.

Also, I was thinking that this is essentially a "welcome" message to the repository of sorts. It might be nice to also use this to recognise and welcome the contributor even more, the same way some other OSSes have decided to work. Something like what scribeOS does? see this reflection

what do @MarkBind/active-devs think?

Edit: I realise this might be confusing bc of my previous comment on the issue, but I'd like to clarify my issue isn't with asking contributors to add themselves, but that the only place where recognition is talked about is asking contributor to add themselves. I personally would find it scary and wonder if a mistake was made, and people might be scared to ask for recognition or feel like their work is not recognised. Other repos which adopt asking contributors to add themselves make it part of the PR checklist and devdocs / readme so that it's clear its part of regular expectations. Otherwise, I think a reminder nudge to the person who merged the PR / active devs is the way to go!

KevinEyo1 · 2024-04-06T03:28:08Z

Hi @kaixin-hc, those are some good points. Can I just clarify what you mean by welcoming them more?

kaixin-hc · 2024-04-06T03:35:22Z

It's an open-ended thought - looking for ideas. I think more encouragement early on can be good for newer contributors? But I'm also not certain on implementation

itsyme · 2024-04-09T08:54:11Z

.github/workflows/new-contributor.yml

+                per_page: 100
+              },
+              (response) => response.data.filter(pr => pr.merged_at != null && pr.user.login === prAuthor && pr.number !== currentPR)
+            );


Do correct me if I am wrong, from my understanding this gets the last 100 pull requests and checks if the user has a merged PR within the last 100 PRs, would this mean that the comment will be sent for a user that hasn't had any merged PRs in the last 100 PRs?

Hi @itsyme, thanks for the review, it actually collects all the PRs, its just that it collects them 100 at a time for the pagination. You can refer to here. github-scripts uses octokit

KevinEyo1 · 2024-04-09T09:04:44Z

Test workflow:
1st time, new contributor, message pops up on merge

2nd time, not new contributor, no message pops up on merge

itsyme

LGTM! Definitely a useful GitHub action for MarkBind's maintainers! Thanks for the work!

github-actions · 2024-04-11T09:38:11Z

@yucheng11122017 Each PR must have a SEMVER impact label, please remember to label the PR properly.

KevinEyo1 and others added 19 commits January 29, 2024 13:25

a

5444909

Revert test changes

a4a4d72

Merge branch 'MarkBind:master' into master

ea3aca7

Merge branch 'MarkBind:master' into master

c0f79a2

Merge branch 'MarkBind:master' into master

45b8846

Merge branch 'MarkBind:master' into master

21628f7

Add how to check if permissions are given

969865f

Merge branch 'master' of https://github.com/MarkBind/markbind

46397e7

Merge branch 'master' of https://github.com/KevinEyo1/markbind

21fa90c

Merge branch 'master' of https://github.com/MarkBind/markbind

0bc66f7

Merge branch 'MarkBind:master' into master

0ebb7cd

Merge branch 'master' of https://github.com/MarkBind/markbind

f798b1d

Merge branch 'master' of https://github.com/MarkBind/markbind

48399bd

Fix merge conflict

cb84513

Merge branch 'master' of https://github.com/MarkBind/markbind

701183b

Merge branch 'master' of https://github.com/MarkBind/markbind

4a83981

Add workflow

85d3b9f

Merge branch 'master' of https://github.com/MarkBind/markbind

7e7cfb7

Merge branch 'master' into 2457-ping-new-contrib

e5619fa

LamJiuFong reviewed Mar 30, 2024

View reviewed changes

KevinEyo1 and others added 5 commits March 31, 2024 12:34

Fix workflow

2c9e6b4

Merge branch 'master' into 2457-ping-new-contrib

de03628

Update workflow

15a926b

Merge branch '2457-ping-new-contrib' of https://github.com/KevinEyo1/…

9a08f81

…markbind into 2457-ping-new-contrib

Update action

79a6363

KevinEyo1 added 2 commits April 2, 2024 12:30

Add echo

2b43560

Add log

64c280d

Merge branch 'master' into 2457-ping-new-contrib

c9c05c2

yucheng11122017 approved these changes Apr 5, 2024

View reviewed changes

yucheng11122017 and others added 3 commits April 5, 2024 13:17

Fix merge conflict

936dadc

Test time

30d31f7

Merge branch '2457-ping-new-contrib' of https://github.com/KevinEyo1/…

6056ba5

…markbind into 2457-ping-new-contrib

KevinEyo1 and others added 4 commits April 5, 2024 13:28

Remove logs

88405dd

Merge branch 'master' into 2457-ping-new-contrib

cfeffed

Revert "Fix merge conflict"

5b9e7d7

This reverts commit 936dadc.

Merge branch 'master' into 2457-ping-new-contrib

c092296

yucheng11122017 force-pushed the master branch 3 times, most recently from cb84513 to 69ec838 Compare April 5, 2024 06:21

Merge branch 'master' into 2457-ping-new-contrib

30996b3

kaixin-hc reviewed Apr 5, 2024

View reviewed changes

KevinEyo1 and others added 3 commits April 7, 2024 13:08

Merge branch 'master' into 2457-ping-new-contrib

d6324a3

Update files

7b5d6a9

Improve security and upadte message

b695c9b

itsyme reviewed Apr 9, 2024

View reviewed changes

Merge branch 'master' into 2457-ping-new-contrib

6db70bc

itsyme approved these changes Apr 9, 2024

View reviewed changes

KevinEyo1 and others added 4 commits April 10, 2024 13:02

Merge branch 'master' into 2457-ping-new-contrib

8832b1a

Revert to version number

7c00b6e

Rename variable

7e26f6f

Merge branch 'master' into 2457-ping-new-contrib

03bde6f

yucheng11122017 merged commit 889836c into MarkBind:master Apr 11, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a reminder when contributor is new to ping all contributor bot #2484

Add a reminder when contributor is new to ping all contributor bot #2484

KevinEyo1 commented Mar 29, 2024 •

edited

Loading

codecov bot commented Mar 29, 2024 •

edited

Loading

LamJiuFong Mar 30, 2024

KevinEyo1 Mar 30, 2024 •

edited

Loading

LamJiuFong Apr 1, 2024

KevinEyo1 Apr 2, 2024

LamJiuFong Apr 2, 2024

itsyme Apr 9, 2024

yucheng11122017 Apr 9, 2024

yucheng11122017 commented Apr 1, 2024

KevinEyo1 commented Apr 2, 2024

yucheng11122017 left a comment

KevinEyo1 commented Apr 5, 2024

kaixin-hc left a comment •

edited

Loading

KevinEyo1 commented Apr 6, 2024

kaixin-hc commented Apr 6, 2024

itsyme Apr 9, 2024 •

edited

Loading

KevinEyo1 Apr 9, 2024 •

edited

Loading

KevinEyo1 commented Apr 9, 2024

itsyme left a comment

github-actions bot commented Apr 11, 2024

Add a reminder when contributor is new to ping all contributor bot #2484

Add a reminder when contributor is new to ping all contributor bot #2484

Conversation

KevinEyo1 commented Mar 29, 2024 • edited Loading

codecov bot commented Mar 29, 2024 • edited Loading

Codecov Report

LamJiuFong Mar 30, 2024

Choose a reason for hiding this comment

KevinEyo1 Mar 30, 2024 • edited Loading

Choose a reason for hiding this comment

LamJiuFong Apr 1, 2024

Choose a reason for hiding this comment

KevinEyo1 Apr 2, 2024

Choose a reason for hiding this comment

LamJiuFong Apr 2, 2024

Choose a reason for hiding this comment

itsyme Apr 9, 2024

Choose a reason for hiding this comment

yucheng11122017 Apr 9, 2024

Choose a reason for hiding this comment

yucheng11122017 commented Apr 1, 2024

KevinEyo1 commented Apr 2, 2024

yucheng11122017 left a comment

Choose a reason for hiding this comment

KevinEyo1 commented Apr 5, 2024

kaixin-hc left a comment • edited Loading

Choose a reason for hiding this comment

KevinEyo1 commented Apr 6, 2024

kaixin-hc commented Apr 6, 2024

itsyme Apr 9, 2024 • edited Loading

Choose a reason for hiding this comment

KevinEyo1 Apr 9, 2024 • edited Loading

Choose a reason for hiding this comment

KevinEyo1 commented Apr 9, 2024

itsyme left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 11, 2024

KevinEyo1 commented Mar 29, 2024 •

edited

Loading

codecov bot commented Mar 29, 2024 •

edited

Loading

KevinEyo1 Mar 30, 2024 •

edited

Loading

kaixin-hc left a comment •

edited

Loading

itsyme Apr 9, 2024 •

edited

Loading

KevinEyo1 Apr 9, 2024 •

edited

Loading