Skip to content
This repository has been archived by the owner on Mar 14, 2023. It is now read-only.

Existing contributors getting welcome messages #4

Closed
nrc opened this issue Sep 24, 2014 · 6 comments
Closed

Existing contributors getting welcome messages #4

nrc opened this issue Sep 24, 2014 · 6 comments

Comments

@nrc
Copy link
Member

nrc commented Sep 24, 2014

e.g. rust-lang/rust#17508 got a comment but the author has previously submitted rust-lang/rust#17286

cc @huonw

My guess would be a mismatch between the Git and GitHub user info, but who knows?

@jdm
Copy link
Contributor

jdm commented Sep 24, 2014

Yeah, it goes by email addresses and https://api.github.com/users/vberger doesn't have one specified, while the PR does use one.

@huonw
Copy link
Member

huonw commented Sep 24, 2014

More data:

(https://api.github.com/users/MatejLach vs. https://github.com/rust-lang/rust/pull/17510.patch)

I have a feeling the current implementation isn't quite enough for the high traffic Rust repo, at least compared to Servo, e.g. the contributors API endpoint is only returning the top 100 (despite the per_page=400, which isn't quite enough anyway, we currently have 614!), meaning new contributors will get welcome messages until they've landed ~20 commits. One possible way to resolve this would be to store the emails from the git history; but statefulness is sad. :(

@jdm
Copy link
Contributor

jdm commented Sep 24, 2014

Actually I was wrong; it's not email but github-login based, but it's definitely the fact that it's only pulling the top 100 contributors that's messing us up here.

@nrc
Copy link
Member Author

nrc commented Sep 25, 2014

I hope I fixed this by handling pagination properly. I guess there might still be issues with username mismatch. Lets see if that is a problem...

@nrc
Copy link
Member Author

nrc commented Sep 25, 2014

Hopefully fixed by fcffc5d I'll leave this issue open for a while and close it if we get no new instances of this.

@nrc
Copy link
Member Author

nrc commented Nov 3, 2014

Haven't seen any false positives recently, so closing.

@nrc nrc closed this as completed Nov 3, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants