[hibernate-dev] Relaxing the pull request review requirement

Gunnar Morling gunnar at hibernate.org
Wed May 1 13:49:34 EDT 2013


At least in HV we've been doing this effectively already, e.g. fixed typos
without creating a PR. In particular when doing a review of a PR, merging
this and adding another commit with this sort of tiny fixes, without
creating a new PR for this.

So I think this makes sense in some situtations, but IMO it really should
be a very rarely used exception. Otherwise there might be the danger that
the limit of what is considered small enough to self-merge slowly rises of
time.

I think the root question really is why do peer reviews lead to "painfully
slow integrations".

Are you regularly waiting for your PRs to be integrated? Then maybe the
possible reviewers should make it a higher prio to do the reviews?
Personally, I try to do any PR reviews with a very high prio, i.e. bring my
current task to a state where I can do a task change and do the review.

> but I'd propose that if you're confident it
is simple enough, the review done by the bot should be considered good
enough.

What kind of "review" is the bot doing in addition to running the build?

--Gunnar




2013/5/1 Sanne Grinovero <sanne at hibernate.org>

> We have been generally very strict in requiring any code change to be
> reviewed by someone else;
> I still believe this has many benefits, but also it brings
> occasionally to painfully slow integrations for trivial fixed.
>
> Since we have a bot now automatically verifying changes, I think this
> rule should be relaxed a bit; still highly recommended to wait for
> someone else to merge it, but I'd propose that if you're confident it
> is simple enough, the review done by the bot should be considered good
> enough.
>
> I just self-merged :
> https://github.com/hibernate/hibernate-search/pull/406
>
> hope you all agree that waiting for feedback was unnecessary?
>
> Of course this should be used very sparingly, also considering that
> even if you think it's correct and simple enough someone might not
> agree with the change, and rises the committer's responsibility to the
> highest level.
>
> Sanne
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>


More information about the hibernate-dev mailing list