patchbot+nagbot

9 views
Skip to first unread message

Simon King

unread,
Dec 29, 2010, 11:58:16 AM12/29/10
to sage-devel
Hi!

I just noticed that my patch from #10296 (ready for review, improving
the communication with singular via pexpect) had bit rotted.

Of course, the patchbot knew that the old patch did not apply to the
current Sage version. Could the patchbot be modified à la "nagbot" and
inform the author of a patch if it fails to apply?

Cheers,
Simon

Robert Bradshaw

unread,
Dec 29, 2010, 1:52:49 PM12/29/10
to sage-...@googlegroups.com

That would be nice. I've been thinking about making it add a note to
trac, so every relevant person on trac would be notified. (Or I could
harvest the nagbot emails addresses, but that would be a pain to keep
up to date).

- Robert

daveloeffler

unread,
Dec 30, 2010, 4:21:26 AM12/30/10
to sage-devel


On Dec 29, 6:52 pm, Robert Bradshaw <rober...@math.washington.edu>
wrote:
> On Wed, Dec 29, 2010 at 8:58 AM, Simon King <simon.k...@uni-jena.de> wrote:
> > Hi!
>
> > I just noticed that my patch from #10296 (ready for review, improving
> > the communication with singular via pexpect) had bit rotted.
>
> > Of course, the patchbot knew  that the old patch did not apply to the
> > current Sage version. Could the patchbot be modified à la "nagbot" and
> > inform the author of a patch if it fails to apply?

I can see a slight problem with this. At present there's no mechanism
to explain to the patchbot exactly which patches to apply. So if you
have (say) a ticket with a patch that requires a patch from an earlier
ticket to be applied, or if you have three patches of which only the
last two should be applied (because the first was a superseded
attempt), or any other nontrivial situation, the patchbot will be
sending out loads of unnecessary messages.

David

Simon King

unread,
Dec 30, 2010, 5:01:18 AM12/30/10
to sage-devel
Hi Dave,

On 30 Dez., 10:21, daveloeffler <dave.loeff...@gmail.com> wrote:
> I can see a slight problem with this. At present there's no mechanism
> to explain to the patchbot exactly which patches to apply. So if you
> have (say) a ticket with a patch that requires a patch from an earlier
> ticket to be applied, or if you have three patches of which only the
> last two should be applied (because the first was a superseded
> attempt), or any other nontrivial situation, the patchbot will be
> sending out loads of unnecessary messages.

I may be mistaken, but as far as I know, if ticket #9876 has 4 patches
A.patch,...,D.patch, of which only D, B and C are to be applied, after
applying the patches from ticket #9999 and #8888, you simply need to
provide a comment that contains the lines

depends on #9999, #8888
apply D.patch, B.patch, A.patch

So, the patchbot would only send out a message if one forgot to
indicate how the patches should be applied (which is an important
information anyway).

Cheers,
Simon

daveloeffler

unread,
Dec 30, 2010, 7:13:05 AM12/30/10
to sage-devel
I'm sorry -- I clearly haven't been keeping pace with developments on
the patchbot! It gets more and more useful every day.

David

Robert Bradshaw

unread,
Dec 30, 2010, 12:41:48 PM12/30/10
to sage-...@googlegroups.com

And otherwise it does a "best guess" kind of approach, which is decent
(especially if there is only one patch :). In any case, I don't think
people would mind getting a "nag" that the patchbot got confused,
indicating that it might be worth your while to give it a hint.

- Robert

daveloeffler

unread,
Dec 30, 2010, 5:04:05 PM12/30/10
to sage-devel
On Dec 30, 1:41 pm, Robert Bradshaw <rober...@math.washington.edu>
wrote:
>
> And otherwise it does a "best guess" kind of approach, which is decent
> (especially if there is only one patch :). In any case, I don't think
> people would mind getting a "nag" that the patchbot got confused,
> indicating that it might be worth your while to give it a hint.
>
> - Robert

Agreed. Are there docs for the patchbot extant somewhere? It would be
handy to have some brief guidelines (perhaps on the Trac wiki?)

David

Robert Bradshaw

unread,
Dec 30, 2010, 5:16:16 PM12/30/10
to sage-...@googlegroups.com

Marco Streng

unread,
Dec 30, 2010, 6:32:47 PM12/30/10
to sage-...@googlegroups.com

William Stein

unread,
Jan 1, 2011, 1:29:14 AM1/1/11
to sage-...@googlegroups.com

This would be done by modifying
SAGE_ROOT/devel/sage/doc/en/developer/trac.rst, then making a patch
and posting the patch to
a ticket at http://trac.sagemath.org/sage_trac/. I'm sure you could
easily do all of this. Thanks!

William

Reply all
Reply to author
Forward
0 new messages