+1 to the general sentiment of trying to get more eyes on a larger change
before committing.
We could start making a more conscious decision of calling out these
changes and then seeing if it's gets any attention
I don't have any tool preference so I started with review board for my
latest patch
> This seems to be what the majority thinks and I see the point, I’m
concerned of this myself. I’m just not sure how to encourage people to
submit for review and review other peoples more since the option is there
now and is not very frequently used. I’m open to suggestions if anyone has
ideas.
It
> Like Dawid I hope we won't add strict requirements to get changes reviewed
> before merging but I do agree with the general sentiment that reviews are
> helpful and improve code quality.
This seems to be what the majority thinks and I see the point, I’m concerned of
this myself. I’m just not
Like Dawid I hope we won't add strict requirements to get changes reviewed
before merging but I do agree with the general sentiment that reviews are
helpful and improve code quality. I really appreciate getting feedback on
patches that I upload, including negative feedback and I don't mind being
pi
On Wed, Feb 28, 2018 at 1:58 PM, Shawn Heisey wrote:
>
> I notice in ZK issues that projects associated with Hadoop have an
> *automatic* machine-generated QA check whenever a patch is submitted on
> those projects. This obviously is not the same as a real review by a
> person, but the info it o
On 2/28/2018 10:59 AM, Tomas Fernandez Lobbe wrote:
> In an effort to improve code quality, I’d like to suggest that we
> start requiring code review to non-trivial patches. Not sure if/how
> other open source projects are doing code reviews, but I’ve been using
> it in internal projects for many y
> I’d like to suggest that we start requiring code review to non-trivial
> patches.
Don't know if it has to be a strict, corporate-like rule... Most folks
over here do get the gut feeling on what's non-trivial and requires a
second pair of eyes. JIRA and patch reviews have been serving this
purpo
> To add to it, I think we should also wait before merging things to the
stable branch and commit only to master in case of non-trivial patches.
Maybe sometimes; a judgement call. It can draw out how long it takes for
issues to get to completion; making it easier to forget that an issue isn't
qui
+1 to the idea of code review before committing non-trivial patches.
I do however worry about the cases when someone asks for feedback but doesn’t
hear from anyone for reasonably long durations. In such situations perhaps a
week should be good enough time to ask for feedback and wait before mer
Ok, so it's clear what you're proposing then. You want to change the CTR
policy. That is indeed quite a big proposal. As I mentioned I'm personally
for CTR, but it would be good to hear other peoples thoughts on this.
Joel Bernstein
http://joelsolr.blogspot.com/
On Wed, Feb 28, 2018 at 1:30 PM, T
I’m not sure how CTR was put in place either, but it was done 10+ years ago,
when Solr had less than 1/10 of the committers it has now and who knows how
many less production deployments/users. Now Solr is a completely different
project than back then, and what was the correct process then may no
I agree that code reviews would be a good idea. But to require code reviews
before committing would be a big change in practice for the Solr
committers. I'm not sure how the commit, then review policy was put in
place or what it would mean to change that. Also I would probably
personally vote again
+1 I'm comfortable with that. And I don't think this rule should apply to
Solr alone; it should apply to Lucene as well, even though a greater
percentage of issues there get reviews.
I think we all appreciate the value of code reviews -- no convincing of
that needed. The challenge this will cre
13 matches
Mail list logo
