At #13370, I just noticed that the patchbot isn't even running the
doctest, because plugins.coverage complains. I think it is a bad idea to
not run the tests just because plugins.commit_messages or
plugins.coverage or plugins.trailing_whitespace fails.
In addition, I don't understand why the plugin is complaining: All
functions introduced by my patch have a test, and I even add a test that
refers to the ticket (showin that some problem is fixed). So, what
criteria are to meet for making the plugin happy?
On Fri, Aug 31, 2012 at 10:04 AM, Simon King <simon.k...@uni-jena.de> wrote:
> Hi!
Hi. Yeah, I've been making a lot of improvements over the last couple
of days. Glad to see someone noticed :).
> At #13370, I just noticed that the patchbot isn't even running the
> doctest, because plugins.coverage complains. I think it is a bad idea to
> not run the tests just because plugins.commit_messages or
> plugins.coverage or plugins.trailing_whitespace fails.
Yes, very true. This particular patchbot is running in a "plugin only"
mode for testing and quick coverage; the normal behavior has not
changed.
> In addition, I don't understand why the plugin is complaining: All
> functions introduced by my patch have a test, and I even add a test that
> refers to the ticket (showin that some problem is fixed). So, what
> criteria are to meet for making the plugin happy?
Decreased doctests misc/cachefunc.pyx from 56 / 63 = 88% to 60 / 68 = 88%
which indicates that there were formerly 7 undoctested functions in
this file and now there are 8. Now it's quite possible that sage
-coverageall has a bug in it, which is beyond the scope of the
patchbot.
Also, unhappy plugins shouldn't be a blocker, rather they should be
something the reviewer should at least be aware of (which may of
course indicate a blocker, if there are for example new undoctested
functions).
On 2012-08-31, Robert Bradshaw <rober...@math.washington.edu> wrote:
> On Fri, Aug 31, 2012 at 10:04 AM, Simon King <simon.k...@uni-jena.de> wrote:
>> In addition, I don't understand why the plugin is complaining: All
>> functions introduced by my patch have a test, and I even add a test that
>> refers to the ticket (showin that some problem is fixed). So, what
>> criteria are to meet for making the plugin happy?
On Fri, Aug 31, 2012 at 1:39 PM, Simon King <simon.k...@uni-jena.de> wrote:
> Hi Robert,
> On 2012-08-31, Robert Bradshaw <rober...@math.washington.edu> wrote:
>> On Fri, Aug 31, 2012 at 10:04 AM, Simon King <simon.k...@uni-jena.de> wrote:
>>> In addition, I don't understand why the plugin is complaining: All
>>> functions introduced by my patch have a test, and I even add a test that
>>> refers to the ticket (showin that some problem is fixed). So, what
>>> criteria are to meet for making the plugin happy?
>> Decreased doctests misc/cachefunc.pyx from 56 / 63 = 88% to 60 / 68 = 88%
>> which indicates that there were formerly 7 undoctested functions in
>> this file and now there are 8.
> The point is that my patch doesn't touch misc/cachefunc.pyx.
I see that you have 5 dependencies; perhaps one of them does? (It
computes the diff against pristine sage-main, dependencies and all.)
It doesn't try to build/run/test the union of your dependencies minus
the new patches (though that's arguably a sane feature request).
On 2012-08-31, Robert Bradshaw <rober...@math.washington.edu> wrote:
>> The point is that my patch doesn't touch misc/cachefunc.pyx.
> I see that you have 5 dependencies; perhaps one of them does?
The plugin compares sage-5.3.rc0 with sage-5.3.rc0+#13370. Two of the five
dependencies of the ticket are already part of sage-5.3.rc0. With the
dependencies of the dependencies, we have to consider #715, #11521,
#12215 and #12313.
#715 doesn't touch misc/cachefunc.pyx. Neither does #11521 or #12313.
#12215 introduces a new class in nisc/cachefunc. Both the class itself and its
methods `__init__`, `__call__`, `is_in_cache` and `set_cache` are
tested. I could imagine, though, that the plugin does not recognise all
of the tests: The class is called WeakCachedFunction, but it gives rise
to a decorator called weak_cached_function.
On Fri, Aug 31, 2012 at 2:41 PM, Simon King <simon.k...@uni-jena.de> wrote:
> Hi Robert,
> On 2012-08-31, Robert Bradshaw <rober...@math.washington.edu> wrote:
>>> The point is that my patch doesn't touch misc/cachefunc.pyx.
>> I see that you have 5 dependencies; perhaps one of them does?
> The plugin compares sage-5.3.rc0 with sage-5.3.rc0+#13370. Two of the five
> dependencies of the ticket are already part of sage-5.3.rc0. With the
> dependencies of the dependencies, we have to consider #715, #11521,
> #12215 and #12313.
> #715 doesn't touch misc/cachefunc.pyx. Neither does #11521 or #12313.
> #12215 introduces a new class in nisc/cachefunc. Both the class itself and its
> methods `__init__`, `__call__`, `is_in_cache` and `set_cache` are
> tested. I could imagine, though, that the plugin does not recognise all
> of the tests: The class is called WeakCachedFunction, but it gives rise
> to a decorator called weak_cached_function.