[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Fast-track for commits with unit tests
From:       Johannes Simon <johannes.simon () gmail ! com>
Date:       2010-09-03 19:17:12
Message-ID: B3E7DD21-48AB-4092-84F0-4795D067FFCE () gmail ! com
[Download RAW message or body]

I agree that it would speed up the process and, in some cases, a unit test is the \
only thing that a patch misses, but during most reviews, more than the question of a \
unit test arises. There's several things that can only be verified or improved on \
                reviewboard, including
- a certain "sanity check" of the code: does it make sense? Does it not just add \
                another corner case to fix the bug?
- readability of the code (comments, docs, magic numbers, etc.)
- is the fix in the correct place?
- whitespace and coding style errors

I think it should rather be considered a good practice to add a unit test to a review \
:-)

Johannes

Am 03.09.2010 um 18:59 schrieb Cyrille Berger:

> Hi,
> 
> I often hear that the use of reviewboard is slowing down the work. And that we 
> lack unit tests. And many review requests show in comment and request for an 
> unit test.
> 
> I would therefor suggest that we add to our reviewboard policy [1], the 
> possibility to commit directly, without a need for prereview, in case, the 
> change comes with a unit test, that show how the previous behavior was wrong 
> and fix it. And of course, under the condition that the fix does not break any 
> other unit test.
> 
> Any thought ?
> 
> [1] http://wiki.koffice.org/index.php?title=Policies/Review_board_rules
> -- 
> Cyrille Berger
> _______________________________________________
> koffice-devel mailing list
> koffice-devel@kde.org
> https://mail.kde.org/mailman/listinfo/koffice-devel

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic