code ownership

[Adam Heath-2]

So, with the recent threads we've participated in, I've decided to
announce a decision of mine.

Any class that I have added test cases that have gotten to 100%
coverage, I have decided to take ownership of.  Any change that is
done to such a class *must* have a test case that keeps the coverage
at 100%.  This doesn't mean that the code will automatically work
perfectly all the time.  But if something *is* added that makes the
coverage fall below 100%, it means the class is *not* fully tested,
and that is a big problem.

The classes that fall into this category are:

org.ofbiz.base.concurrent.TTLCachedObject
org.ofbiz.base.json.JSONWriter
org.ofbiz.base.lang.ComparableRange
org.ofbiz.base.util.IndentingWriter
org.ofbiz.base.util.TimeDuration
org.ofbiz.base.util.collections.FlexibleMapAccessort
org.ofbiz.base.util.string.FlexibleStringExpander

The above also have 100% coverage in their tests.

The following classes are not fully covered, because of either
difficulty in throwing exceptions at the right place, or non-blocking
algorithms that are hard to write correct multi-threadeds tests for.
The numbers stand for line coverage/branch coverage.

org.ofbiz.base.concurrent.TTLObject(91%/94%)
org.ofbiz.base.conversion.Converters(89%/86%)
org.ofbiz.base.util.UtilObject(93%/100%)

org.ofbiz.base.util.ObjectType.simpleTypeConvert has full line
coverage.  The only thing not covered is a single branch, the one at
the end of the method that calls Debug.infoOn().

These additional 3 classes and method I am also going to take
ownership of.

While Converters has good coverage, the actual converter
implementations do not.  This is something I will be fixing in the
coming days.

[Anil Patel-3]

Adam,
Thanks for hard work. I am sure people in community respect your effort and will co-operate.
 
Thanks and Regards
Anil Patel
HotWax Media Inc
Find us on the web at www.hotwaxmedia.com or Google Keyword "ofbiz"

On Mar 15, 2010, at 12:40 AM, Adam Heath wrote:

> So, with the recent threads we've participated in, I've decided to
> announce a decision of mine.
>
> Any class that I have added test cases that have gotten to 100%
> coverage, I have decided to take ownership of.  Any change that is
> done to such a class *must* have a test case that keeps the coverage
> at 100%.  This doesn't mean that the code will automatically work
> perfectly all the time.  But if something *is* added that makes the
> coverage fall below 100%, it means the class is *not* fully tested,
> and that is a big problem.
>
> The classes that fall into this category are:
>
> org.ofbiz.base.concurrent.TTLCachedObject
> org.ofbiz.base.json.JSONWriter
> org.ofbiz.base.lang.ComparableRange
> org.ofbiz.base.util.IndentingWriter
> org.ofbiz.base.util.TimeDuration
> org.ofbiz.base.util.collections.FlexibleMapAccessort
> org.ofbiz.base.util.string.FlexibleStringExpander
>
> The above also have 100% coverage in their tests.
>
> The following classes are not fully covered, because of either
> difficulty in throwing exceptions at the right place, or non-blocking
> algorithms that are hard to write correct multi-threadeds tests for.
> The numbers stand for line coverage/branch coverage.
>
> org.ofbiz.base.concurrent.TTLObject(91%/94%)
> org.ofbiz.base.conversion.Converters(89%/86%)
> org.ofbiz.base.util.UtilObject(93%/100%)
>
> org.ofbiz.base.util.ObjectType.simpleTypeConvert has full line
> coverage.  The only thing not covered is a single branch, the one at
> the end of the method that calls Debug.infoOn().
>
> These additional 3 classes and method I am also going to take
> ownership of.
>
> While Converters has good coverage, the actual converter
> implementations do not.  This is something I will be fixing in the
> coming days.

[Adam Heath-2]

Anil Patel wrote:
> Adam,
> Thanks for hard work. I am sure people in community respect your effort and will co-operate.

This decision of mine doesn't preclude others from doing work in these
classes.  But I will be watching these classes like a hawk, and would
greatly appreciate anyone that wants to change them try to keep the
coverage up if they do so.

[Adrian Crum-2]

Thanks Adam! I'm trying to include tests in my commits. It would be cool if others did the same.

-Adrian

--- On Sun, 3/14/10, Adam Heath <[hidden email]> wrote:

> From: Adam Heath <[hidden email]>
> Subject: code ownership
> To: [hidden email]
> Date: Sunday, March 14, 2010, 9:40 PM
> So, with the recent threads we've
> participated in, I've decided to
> announce a decision of mine.
>
> Any class that I have added test cases that have gotten to
> 100%
> coverage, I have decided to take ownership of.  Any
> change that is
> done to such a class *must* have a test case that keeps the
> coverage
> at 100%.  This doesn't mean that the code will
> automatically work
> perfectly all the time.  But if something *is* added
> that makes the
> coverage fall below 100%, it means the class is *not* fully
> tested,
> and that is a big problem.
>
> The classes that fall into this category are:
>
> org.ofbiz.base.concurrent.TTLCachedObject
> org.ofbiz.base.json.JSONWriter
> org.ofbiz.base.lang.ComparableRange
> org.ofbiz.base.util.IndentingWriter
> org.ofbiz.base.util.TimeDuration
> org.ofbiz.base.util.collections.FlexibleMapAccessort
> org.ofbiz.base.util.string.FlexibleStringExpander
>
> The above also have 100% coverage in their tests.
>
> The following classes are not fully covered, because of
> either
> difficulty in throwing exceptions at the right place, or
> non-blocking
> algorithms that are hard to write correct multi-threadeds
> tests for.
> The numbers stand for line coverage/branch coverage.
>
> org.ofbiz.base.concurrent.TTLObject(91%/94%)
> org.ofbiz.base.conversion.Converters(89%/86%)
> org.ofbiz.base.util.UtilObject(93%/100%)
>
> org.ofbiz.base.util.ObjectType.simpleTypeConvert has full
> line
> coverage.  The only thing not covered is a single
> branch, the one at
> the end of the method that calls Debug.infoOn().
>
> These additional 3 classes and method I am also going to
> take
> ownership of.
>
> While Converters has good coverage, the actual converter
> implementations do not.  This is something I will be
> fixing in the
> coming days.
>

[David E. Jones-2]

Adam,

Your second email sounds more like code monitoring than ownership. Is that more of what you had in mind?

As far as whether these constraints are considered a policy or an obligation for other committers, I guess we'd have to vote on that. I don't think we have anything setup (yet) to allow individuals to create obligations for others.

Actually, even if we vote on something that creates obligations for others what can we really do about it? I guess we can get into commit wars, but that's about it. We don't have men with guns, or cages to put people in, or even people who write threatening letters for a living.

As a labour saving device you might consider putting some information in these files themselves so people have a chance to collaborate with you on this, and you don't have to worry so much about watching it, especially a year or two from now when this thread has faded from even our fondest dreams. Useful information might include the coverage status, where the tests are, how to run and test for coverage, etc (I think all of these would be one-liners in the file).

-David


On Mar 14, 2010, at 10:40 PM, Adam Heath wrote:

> So, with the recent threads we've participated in, I've decided to
> announce a decision of mine.
>
> Any class that I have added test cases that have gotten to 100%
> coverage, I have decided to take ownership of.  Any change that is
> done to such a class *must* have a test case that keeps the coverage
> at 100%.  This doesn't mean that the code will automatically work
> perfectly all the time.  But if something *is* added that makes the
> coverage fall below 100%, it means the class is *not* fully tested,
> and that is a big problem.

On Mar 14, 2010, at 10:55 PM, Adam Heath wrote:

> Anil Patel wrote:
>> Adam,
>> Thanks for hard work. I am sure people in community respect your effort and will co-operate.
>
> This decision of mine doesn't preclude others from doing work in these
> classes.  But I will be watching these classes like a hawk, and would
> greatly appreciate anyone that wants to change them try to keep the
> coverage up if they do so.

[Adam Heath-2]

David E Jones wrote:
> Adam,
>
> Your second email sounds more like code monitoring than ownership. Is that more of what you had in mind?

Hm, yes, monitor might be a better term.

> As far as whether these constraints are considered a policy or an obligation for other committers, I guess we'd have to vote on that. I don't think we have anything setup (yet) to allow individuals to create obligations for others.

I wouldn't use a sledgehammer.  It would always be a training class,
whenever something changed that causes coverage to drop.  Eventually I
would hope others learn what I have tried to do here.

The reason someone writes code, is that they are trying to solve a
need.  Saying "Take this code and shove it" is wrong.  It is obviously
useful.  So, you have to work with what they have tried to do.

(ps: I am not saying you said anything like this, just talking in
generalities)

> As a labour saving device you might consider putting some information in these files themselves so people have a chance to collaborate with you on this, and you don't have to worry so much about watching it, especially a year or two from now when this thread has faded from even our fondest dreams. Useful information might include the coverage status, where the tests are, how to run and test for coverage, etc (I think all of these would be one-liners in the file).

The term you are looking for here is annotations.