Обсуждение: Reviewers needed for pgjdbc pull requests
There are quite a few pull requests on the github project https://github.com/pgjdbc/pgjdbc/pulls
Most of the work fall on Vladimir and I try to do some as well.
I'm looking for suggestions on how to deal with this. Clearly pull requests become stale if they are not dealt with quickly.
One thought is that if you have an existing PR that you want pushed then pick up another to review.
I'm open to suggestions?
Dave Cramer
Hi Dave, I will be happy to help out some if you can tell me little more about your PR acceptance process.
— Zemian
There are quite a few pull requests on the github project https://github.com/pgjdbc/pgjdbc/pullsMost of the work fall on Vladimir and I try to do some as well.I'm looking for suggestions on how to deal with this. Clearly pull requests become stale if they are not dealt with quickly.One thought is that if you have an existing PR that you want pushed then pick up another to review.I'm open to suggestions?Dave Cramer
I believe people (myself included) are simply not aware that them reviewing pull requests would do any good. By definition, pull requests must be reviewed and can only be merged by these which have the authority to do so. So it seems superfluous when somebody like me adds a "looks good to me" comment under a PR. It has to be checked by the merging person anyway. For example, I can review PRs, but I'm not *that* familiar with the code base so I can't guarantee that something doesn't slip past me that might have a large impact. On 15.05.2018 15:45, Dave Cramer wrote: > There are quite a few pull requests on the github project > https://github.com/pgjdbc/pgjdbc/pulls > > Most of the work fall on Vladimir and I try to do some as well. > > I'm looking for suggestions on how to deal with this. Clearly pull requests > become stale if they are not dealt with quickly. > > One thought is that if you have an existing PR that you want pushed then > pick up another to review. > > I'm open to suggestions? > > Dave Cramer >
hmmm well there's a good start since we don't have a PR acceptance process.
Clearly it has to pass the CI tests, but other than that we don't have a clear process.
Sounds like that is step 1.
Are there examples from other projects ?
Dave Cramer
On 15 May 2018 at 10:00, Zemian Deng <zemiandeng@gmail.com> wrote:
Hi Dave, I will be happy to help out some if you can tell me little more about your PR acceptance process.— ZemianThere are quite a few pull requests on the github project https://github.com/pgjdbc/pgjdbc/pulls Most of the work fall on Vladimir and I try to do some as well.I'm looking for suggestions on how to deal with this. Clearly pull requests become stale if they are not dealt with quickly.One thought is that if you have an existing PR that you want pushed then pick up another to review.I'm open to suggestions?Dave Cramer
On Tue, May 15, 2018 at 9:45 AM, Dave Cramer <davecramer@gmail.com> wrote:
There are quite a few pull requests on the github project https://github.com/pgjdbc/pgjdbc/pulls
Wow 72 open and 20+ since Jan of this year. On the flip side the activity level is also pretty impressive!
Most of the work fall on Vladimir and I try to do some as well.
And we thank you both! :D
I'm looking for suggestions on how to deal with this. Clearly pull requests become stale if they are not dealt with quickly.One thought is that if you have an existing PR that you want pushed then pick up another to review.
I wouldn't say imposing it as a rule but *strongly* encouraged would be nice.
I'm open to suggestions?
Might be able to arrange things a bit better with more tagging. There's a bit of it for some of the older PRs but none of the recent ones have any tags applied.
It adds some work on the part of the organizer but may allow more people to get involved as simpler items could be more easily found.
Rather than imposing this as more work on the the repo owners it could be part of the PR template itself. That way the repo owner (or someone else given tagging permissions) could apply them quickly without having to actually review the entire PR.
Having a bit more structure or guidelines for the review process would be nice too. If people know that passing an objective third party review will grant an audience from a committer, I think it'd welcome more reviews and they'd seek those reviews themselves (i.e. find someone to help review their PR).
-S
On Tue, May 15, 2018 at 10:04 AM, Robert Zenz <robert.zenz@sibvisions.com> wrote:
I believe people (myself included) are simply not aware that them reviewing pull
requests would do any good. By definition, pull requests must be reviewed and
can only be merged by these which have the authority to do so. So it seems
superfluous when somebody like me adds a "looks good to me" comment under a PR.
It has to be checked by the merging person anyway.
It's still helpful though. Even simply chiming in to say that a particular feature may be useful without actually reviewing a PR is helpful as it identifies the PRs that reviewers should be focusing on (i.e. stuff that people actually want and will use).
It works in reverse as well. New features mean new code complexity that needs to be maintained over time. Chiming in to say that a feature makes no sense, doesn't cover a edge cases, or that you simply would never use it helps decide if a given change will be worth it in the long run.
-S
"looks good to me" looks good to me.
From my point of view, the following review items are hard to verify automatically:
missing test cases
backward compatibility.
Of course, it is not trivial to have 100% cases covered, so sometimes I just try to check how the code plays against corner cases.
Vladimir