Two beginner questions about patchbot

56 views
Skip to first unread message

Reimundo Heluani

unread,
May 15, 2020, 5:19:11 PM5/15/20
to sage-...@googlegroups.com
Hello, I started today running a patchbot. I wanted to test a couple of very
simple patches #29690 and #29691. The latter depends on the former. #29690
passes without problems but #29691 fails tests as if the patch from #29690
were not applied.

Now here is were I screwed up. Since #29691 needed #29690, before pushing I
had added that change to check that all tests passed. I mistakenly pushed this
and then added a commit simply reverting #29690. So now these two branches
look like this

C --- D (#29691)
/
---A (develop)
\
B (#29690)


Commit C has what #29691 is supposed to have plus the same change as B. Commit
D simply reverts the change from B.

Now if I were to test #29691 I would merge #29690 from that branch which
results in

C --- D --E
/ /
--A /
\ /
B ----

And then I would merge that branch into develop.

But if the patchbot is merging first B into develop and then C+D then B is
undone and that's the failure I see.

Now my two questions are

1) Is patchbot applying B first and then C + D? I couldn't find anything in
patchbot's logfile for #29691 that indicates this, but I suppose it's doing
this.

2) What's the right etiquette do deal with this situation? I could simply
revert everything since we are talking about trivial patches and noone has
pulled them. But still I would want to know the workflow/rules here.


Thanks for your time

R.

signature.asc

Dima Pasechnik

unread,
May 15, 2020, 5:22:45 PM5/15/20
to sage-devel
by the way, one can use GitHub Actions to test patches - just push an appropriate branch to your GitHub fork of Sage, and if Actions are enabled on your repo, you will get it tested on various systems.


--
You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/20200515211900.GA696691%40vertex.

Matthias Koeppe

unread,
May 15, 2020, 5:37:18 PM5/15/20
to sage-devel
On Friday, May 15, 2020 at 2:19:11 PM UTC-7, Reimundo Heluani wrote:
I wanted to test a couple of very
simple patches #29690 and #29691. The latter depends on the former. #29690
passes without problems but #29691 fails tests as if the patch from #29690
were not applied.

If #29691 depends on #29690, its branch should be on top of the branch of that.

On Trac, add #29690 in the Dependencies field so that reviewers know about this.



Reimundo Heluani

unread,
May 15, 2020, 6:35:05 PM5/15/20
to sage-...@googlegroups.com
Thanks, I had already added this as a dependency and I thought that patchbot
would pick this up and pull that branch first. But now that I forced push and
test again it fails with the same problem so I guess patchbot is not pulling
the dependency branch before.

So is my understanding correct that the right workflow is to rebase #29691 on
top of #29690 and push again?

R.

>
>
>
>
>--
>You received this message because you are subscribed to the Google Groups
>"sage-devel" group.
>To unsubscribe from this group and stop receiving emails from it, send an email
>to [1]sage-devel+...@googlegroups.com.
>To view this discussion on the web visit [2]https://groups.google.com/d/msgid/
>sage-devel/6c8d9fa4-f72c-4f37-b58a-e1780dd59ab1%40googlegroups.com.
>
>References:
>
>[1] mailto:sage-devel+...@googlegroups.com
>[2] https://groups.google.com/d/msgid/sage-devel/6c8d9fa4-f72c-4f37-b58a-e1780dd59ab1%40googlegroups.com?utm_medium=email&utm_source=footer

signature.asc

Matthias Koeppe

unread,
May 15, 2020, 6:47:38 PM5/15/20
to sage-devel
On Friday, May 15, 2020 at 3:35:05 PM UTC-7, Reimundo Heluani wrote:
 is my understanding correct that the right workflow is to rebase #29691 on
top of #29690 and push again?

Yes. 

Reimundo Heluani

unread,
May 15, 2020, 7:10:29 PM5/15/20
to sage-...@googlegroups.com
On May 15, Matthias Koeppe wrote:
Thanks! that's the second push -f in a day if this was a large patch that
people had worked on I'd be killed :). I've learned then that the patchbot will
not apply what's listed as dependencies. Perhaps this should be added to the
development guide. I'm only playing until I can submit #29610 which is not
ready for review but will eventually be a large patch that should be
subdivided. I've learned today that it has to be in the form of a tree

R.




>
>
>--
>You received this message because you are subscribed to the Google Groups
>"sage-devel" group.
>To unsubscribe from this group and stop receiving emails from it, send an email
>to [1]sage-devel+...@googlegroups.com.
>To view this discussion on the web visit [2]https://groups.google.com/d/msgid/
>sage-devel/4a17ec11-650b-4e9e-9bb7-ab874b66fcef%40googlegroups.com.
>
>References:
>
>[1] mailto:sage-devel+...@googlegroups.com
>[2] https://groups.google.com/d/msgid/sage-devel/4a17ec11-650b-4e9e-9bb7-ab874b66fcef%40googlegroups.com?utm_medium=email&utm_source=footer

signature.asc

Matthias Koeppe

unread,
May 15, 2020, 7:29:54 PM5/15/20
to sage-devel
On Friday, May 15, 2020 at 4:10:29 PM UTC-7, Reimundo Heluani wrote:
that's the second push -f in a day  if this was a large patch that
people had worked on I'd be killed :). 

Most tickets for Sage are short-lived feature branches with a single author. Rebasing and other forms of rewriting commit history typically are unproblematic operations. 


Reply all
Reply to author
Forward
0 new messages