PR review and merge process

54 views
Skip to first unread message

Valery Yatsynovich

unread,
Feb 21, 2017, 4:21:34 PM2/21/17
to Selenium Developers
How can I find key persons who have permissions to review and merge pull requests?

There is no such information in https://github.com/SeleniumHQ/selenium/blob/master/CONTRIBUTING.md, but I think it could useful to have a list of key persons (possibly, divided by platforms) who can be contacted in person/mentioned directly in case if review of pull request is delayed or missed.

Simon Stewart

unread,
Feb 22, 2017, 11:12:58 AM2/22/17
to selenium-developers
Hi,

Other than just looking at the list of commits around the area you're interested in and picking the person who's been most active?

Simon

On Fri, Feb 17, 2017 at 1:35 PM, Valery Yatsynovich <valeryya...@gmail.com> wrote:
How can I find key persons who have permissions to review and merge pull requests?

There is no such information in https://github.com/SeleniumHQ/selenium/blob/master/CONTRIBUTING.md, but I think it could useful to have a list of key persons (possibly, divided by platforms) who can be contacted in person/mentioned directly in case if review of pull request is delayed or missed.

--
You received this message because you are subscribed to the Google Groups "Selenium Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to selenium-developers+unsub...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/selenium-developers/4c2cfd1a-3f5e-4677-9f9d-7d125fc225e5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Valery Yatsynovich

unread,
Feb 22, 2017, 3:30:49 PM2/22/17
to Selenium Developers
Hi Simon,

It doesn't work, I've tried to act in this way. So I am thinking about other members who is not very active in development, but still may have time to review/merge. But I was not able to find  them.

Thanks,
Valery

Simon Stewart

unread,
Feb 23, 2017, 7:23:03 AM2/23/17
to selenium-developers
Heh. I think people are either active in development or their doing something else entirely --- my experience is that those not committing code don't tend to review things. Maybe that'll change?

I do, however, believe that just asking for a review from someone on GitHub isn't enough. Lots of people don't check those notifications. A better bet is to hop on to IRC and ask someone to have a look at your PR, especially if it's just been sitting there for a while.

Kind regards,

Simon

On Wed, Feb 22, 2017 at 8:09 PM, Valery Yatsynovich <valeryya...@gmail.com> wrote:
Hi Simon,

It doesn't work, I've tried to act in this way. So I am thinking about other members who is not very active in development, but still may have time to review/merge. But I was not able to find  them.

Thanks,
Valery

On Wednesday, February 22, 2017 at 7:12:58 PM UTC+3, Simon Stewart wrote:
Hi,

Other than just looking at the list of commits around the area you're interested in and picking the person who's been most active?

Simon
On Fri, Feb 17, 2017 at 1:35 PM, Valery Yatsynovich <valeryya...@gmail.com> wrote:
How can I find key persons who have permissions to review and merge pull requests?

There is no such information in https://github.com/SeleniumHQ/selenium/blob/master/CONTRIBUTING.md, but I think it could useful to have a list of key persons (possibly, divided by platforms) who can be contacted in person/mentioned directly in case if review of pull request is delayed or missed.

--
You received this message because you are subscribed to the Google Groups "Selenium Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to selenium-developers+unsubscribe...@googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "Selenium Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to selenium-developers+unsub...@googlegroups.com.

Andreas Tolfsen

unread,
Feb 23, 2017, 8:57:42 AM2/23/17
to selenium-...@googlegroups.com
Valery Yatsynovich <valeryya...@gmail.com> writes:

> It doesn't work, I've tried to act in this way. So I am thinking
> about other members who is not very active in development, but still
> may have time to review/merge. But I was not able to find them.

To find who does work in a certain area, it’s useful to look at the
blame or log for the particular file(s) you’re interested in,
irregardless which project you’re working on. Of course it can be hard
to tie real names to GitHub nicknames, but it gives an idea who knows
enough about the code to review it.

Taking the blame log of the py/selenium/webdriver/firefox/options.py
file as example:

% git blame py/selenium/webdriver/firefox/options.py

dfab9272978 (David Burns     2016-03-15 11:15:56 +0000  32) class Options(object):
9706bb7525f (Andreas Tolfsen 2016-10-10 20:17:25 +0100  33)     KEY = "moz:firefoxOptions"
dfab9272978 (David Burns     2016-03-15 11:15:56 +0000  34) 
dfab9272978 (David Burns     2016-03-15 11:15:56 +0000  35)     def __init__(self):
9706bb7525f (Andreas Tolfsen 2016-10-10 20:17:25 +0100  36)         self._binary = None
0e5b18aa882 (Dave Hunt       2017-01-13 16:28:33 +0000  37)         self._preferences = {}
dfab9272978 (David Burns     2016-03-15 11:15:56 +0000  38)         self._profile = None
dfab9272978 (David Burns     2016-03-15 11:15:56 +0000  39)         self._arguments = []
9706bb7525f (Andreas Tolfsen 2016-10-10 20:17:25 +0100  40)         self.log = Log()

Or the git log:

% git shortlog py/selenium/webdriver/firefox/options.py
Andreas Tolfsen (1):
py: new FirefoxDriver ctor precedence logic and moz:firefoxOptions support (#2882)

Daniel Davison (1):
organize imports of firefox options; expanduser and abspath on chrome options extension (#3089)

Dave Hunt (4):
Avoid overriding capabilities with the defaults when using Firefox options
Correct the expected capability name for the Firefox profile
[py] Fix flake8 issues and run flake8 on Travis
[py] Allow Firefox preferences to be set directly in Options

David Burns (4):
Adding Options object for use with Python FirefoxDriver
Remove unneeded imports in Options module
Pass in an encoded profile to the capabilities created from Options
Move capabilities passed through to be only desiredCapabilities

Valery Yatsynovich

unread,
Mar 4, 2017, 5:08:38 AM3/4/17
to Selenium Developers
Hi Everyone,

Is there some good way to ask for review/merge pull-request in general?

I've tried 
 - pinging people via github
 - asking for review in IRC.
These options don't work for me.

Thanks,
Valery

Simon Stewart

unread,
Mar 4, 2017, 8:51:13 AM3/4/17
to selenium-developers
Hi Valery,

I'm sorry you're not having much luck. It's not because we don't value your PR. On my side, it's because I want to take the time to do a proper review rather than something brief and rushed.

I've skimmed your PR when you asked on the IRC channel and it looks good, but at the time I was heads down in working on the W3C spec and fixing bugs that we'd encountered with some of our releases. Thanks to your diff, I've already done some clean up in ExpectedConditions. I'll take another look at it now.

Simon

--
You received this message because you are subscribed to the Google Groups "Selenium Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to selenium-developers+unsub...@googlegroups.com.

Valery Yatsynovich

unread,
Mar 6, 2017, 2:46:03 PM3/6/17
to Selenium Developers
Hi Simon,

Thank you for your explanation and thank you for merging PR.

To summarize:
 - find active people by commits and try them pinging via github
 - use IRC to request for review
 - be patient :)

Thanks,
Valery

Simon Stewart

unread,
Mar 7, 2017, 3:58:15 AM3/7/17
to selenium-developers
In essence, that's correct. There are some other project members who are significantly better at checking GitHub for PRs to review than I am, so sometimes the active people find you :)

Thank you for being patient, and thank you for the PR!

Simon

--
You received this message because you are subscribed to the Google Groups "Selenium Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to selenium-developers+unsub...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages