Code review comment for lp:~epics-core/epics-base/ioc-shutdown

Ralph Lange (ralph-lange) wrote :

> On 12/03/2013 03:39 PM, mdavidsaver wrote:
> > On 12/03/2013 02:35 PM, Andrew Johnson wrote:
> >> For future reference: With almost 2**10 lines of diff this branch mixes
> > together (admittedly related) changes to several different subsystems,
> >
> > ??? The merge proposal shows:
> >
> > 961 lines (+337/-53) 21 files modified
>
> 961 ≈ 1024
>
> I'm just saying that the longer the diff is, the more reviewers will be
> put off looking at it. If the changes were split into smaller dependent
> branches, reviewing them might look like less work and could be done in
> smaller chunks (this was a human psychology comment, not a programming
> issue at all).

I was considering this, but did not find a useful subset of the changes that would have added a functionality that could be documented and tested for.

Which in my imagination would have led to rejection of each individual branch and you commenting that a single functionality should have been presented as a single branch instead of multiple dependent branches that don't add anything useful by itself. ;-)

Admittedly, I might have been able to split the callback related things off. But still with a high change of not being able to add the mandatory tests.

« Back to merge proposal