>> What is the definition of not fully on topic. I would not suggest a change in
>> class X for a pull request where only class Y and Z are affected. However,
>> if class X is touched and I see a potential improvement I think it can be 
>> considered 
>> being part of the topic. Boy Scout rule number one:" Always leave the 
>> campground 
>> cleaner than you found it."  I truly believe in this one, but of course 
>> sometimes a 
>> potential improvement would have too big of a ripple effect to be pursued.
> 
> I think that's the crux of the disagreement.

To a degree yes.

> Disclaimer, it depends but if the cumulated changes take 5 mins or 3 hours 
> things vary.

There is for sure a limit where we are not talking about keeping the campground 
clean anymore.

> Breaking the flow of a small or medium sized PR can be problematic IMO. 

What is "breaking the flow" of a pull request? Are you saying that just because 
the reviewer 
of the pull request discovered potential points of improvement or suggests 
other fixes, it
breaks the work flow of submitter of the pull request, because he did not get 
an immediate merge?

What's the point of reviewing if we are not able to discuss potential 
improvements. Do you want
feedback or do you want someone to press the merge button? 

If your work really depends on this one particular pull request being merged I 
argue that you should not
have submitted it and do a combined pull request. Or as mentioned before just 
keep on working on
your local branch. You have plenty of choices to proceed and there is no need 
to be blocked on a pull request.

The only problematic case I see is, if there is an immediate release required, 
because an external consumer (e.g Infinispan requiring an updated
version of Search with a specific bug fix). However, that is not what we have 
been talking about here.

The moment you submit a pull request you are saying that you want this work to 
be reviewed and merged
to master. At this point you have to be willing to deal with the feedback. If 
not, the whole procedure becomes pointless.

> The other problem is that we all have a different degree of perfection: what 
> is unacceptable for one is ok for another.

Sure. That's probably a good thing. We also tend to have different strength 
when it comes to certain areas of functionality or type
of problems. That's a good thing imo. It means that sometimes you get comments 
about something you have not thought about.

> We are generally in agreement but that only makes disagreements more 
> energetic ;)

As much as I enjoy this discussion, right now we have 0 open pull request in 
Search. 
So where is the problem?

--Hardy


_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev

Reply via email to