all-contributors / app

🤖 A GitHub App to automate acknowledging contributors to your open source projects
https://allcontributors.org
MIT License
576 stars 150 forks source link

feat: don't create PR when contributor already have contribution type #346

Closed rluvaton closed 3 years ago

rluvaton commented 3 years ago

Closes #345

vercel[bot] commented 3 years ago

@rluvaton is attempting to deploy a commit to the All Contributors Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/all-contributors/app/FGVNmUvjjNbW2pYpNkDkon9upuJp
✅ Preview: https://app-git-fork-rluvaton-feat-dont-create-pr-whe-73fef0.vercel.app

gr2m commented 3 years ago

@rluvaton I've deployed your update as preview to Vercel and updated https://github.com/apps/all-contributors-preview to point to that deployment. Can you install https://github.com/apps/all-contributors-preview on a test repository and see if it resolves your problem? Make sure that you don't have both https://github.com/apps/allcontributors and https://github.com/apps/all-contributors-preview installed on the same repository, otherwise the two will conflict with each other

rluvaton commented 3 years ago

@gr2m I installed it and it doesn't respond to any of my comments. Is there a way to debug it?

gr2m commented 3 years ago

Here is what I see in the logs

image

It might be that the private class field is not supported by the node version used by Vercel

rluvaton commented 3 years ago

Thank you for your help, I fixed it

gr2m commented 3 years ago

can you try again

rluvaton commented 3 years ago

@gr2m Work like a charm!

My tests: https://github.com/rluvaton/all-contributor-playground/pull/5

github-actions[bot] commented 3 years ago

:tada: This PR is included in version 1.13.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

rluvaton commented 3 years ago

@gr2m Don't forget to add me as a contributor 😉 😉

gr2m commented 3 years ago

haha I just waited for the new deployment to finish so I could test the changes in production :D

@all-contributors please add @rluvaton for code, bug

allcontributors[bot] commented 3 years ago

@gr2m

I've put up a pull request to add @rluvaton! :tada:

ouuan commented 3 years ago

I used to use the old behavior to update the README. Now how do I update README without adding a new contributor/contribution type?

rluvaton commented 3 years ago

I'm having a hard time to understand 😬, what do you mean by:

I used to use the old behavior to update the README.

Update with what changes?

ouuan commented 3 years ago

Update with what changes?

For example, manual changes of .all-contributorsrc.

ouuan commented 3 years ago

It was a workaround of #157.

rluvaton commented 3 years ago

And you want your manual change to update the README?

ouuan commented 3 years ago

I think the best solution is to implement what #157 requests.

rluvaton commented 3 years ago

I see, I'm a little bit busy at the moment so I won't be able to create a PR, but you're more than welcome 🤗 , I'll try to help you;

From what I understand from the code, you need to add the action here:

https://github.com/all-contributors/app/blob/00d277fec6dad3b0cbe097f8d9faa446e8f380cf/lib/process-issue-comment.js#L30

Extract the code after to an add function

Add the action to here: https://github.com/all-contributors/app/blob/00d277fec6dad3b0cbe097f8d9faa446e8f380cf/lib/parse-comment.js#L197

Take some of the code from here:

https://github.com/all-contributors/app/blob/00d277fec6dad3b0cbe097f8d9faa446e8f380cf/lib/add-contributor.js#L39

And I think that's it 😅

brosaplanella commented 3 years ago

I have noticed that if the bot is asked to add a contributor for a given contribution and the PR is not merged, when asked again the bot says that the user has already contributed before to that aspect. Is this the intended behaviour or is this a bug?

rluvaton commented 3 years ago

I think you should open a new issue and reference this and I try to answer it there