-
Notifications
You must be signed in to change notification settings - Fork 270
Infer reviewers from recents commits authors #168
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Is it just me or is it ironic that @mention-bot is not activated for this repository |
I'm worried about the "stop using blame webpages to find reviewers". Wouldn't this mean that mention-bot would stop producing context-aware reviewer suggestions, based on who has actually worked the code in question? |
Hey @cyberdelia, sorry for the super late response. I do not think that abandoning the blame is the right approach, but you make an excellent point with "also tend to give reviewers that have a overall more "fresher" point of view on the code being changed". In the fb-internal version of mention-bot we are now applying a coefficient based on time on every line of code. If the line of code has been written more than 8 months ago, we ignore it entirely. Then, we do a linear interpolation for 8 months being 0 to now being worth 350 points. This is working much better and I would love to accept a pull request that implements it on this project. @StephanBijzitter mention-bot is activated on this project. The way it is implemented is that there's a fixed buffer size for the http request and if the pull request you are requesting is bigger than that it crashes and drops the request. This is not ideal and a pull request to make it read whatever has been downloaded within that fixed size would be appreciated. However, in practice, this only happens for giant pull requests that change everything, which are going to be highly debated and the people involved are going to see it anyway. I'm going to go ahead and close this pull request as I don't think it's worth losing all the logic. But I'm glad that you were able to use this project as a basis in order to reimplement it in a way that suits your needs and thanks for all the time and efforts you've spent on it! |
@vjeux Do you think there will be another way we can use this bot whilst still enforcing 2FA for all organization members? |
We should get GitHub to provide support for accessing the blame information through their API. Here's a random attempt: https://twitter.com/Vjeux/status/783783705722494977 By the way, feel free to use this pull request for your use case, that's the great thing about open source :) |
This stop using blame webpages to find reviewers, and starts using recent top committers on the different files of the PR. This has the advantage to work with 2FA on and requires a lot less code, it also tend to give reviewers that have a overall more "fresher" point of view on the code being changed.
It will fix #147, #166 and #76.