Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excise MoinMoin #12713

Closed
kini opened this issue Mar 20, 2012 · 36 comments
Closed

Excise MoinMoin #12713

kini opened this issue Mar 20, 2012 · 36 comments

Comments

@kini
Copy link
Contributor

kini commented Mar 20, 2012

We should get rid of MoinMoin as a standard spkg. It is not used by anything in Sage AFAIK. (There has been some discussion about this on sage-devel.) In particular, there was a vote in which (as of March 28) no one objected to removing MoinMoin. There wasn't great consensus about what to do with the package afterwards, but making it "experimental" has support and seems like the right choice: then it is still available but no one has to commit to be the maintainer.

To apply:

Depends on #10492
Depends on #12515

Component: packages: standard

Author: John Palmieri

Reviewer: Jeroen Demeyer

Merged: sage-5.0.beta13

Issue created by migration from https://trac.sagemath.org/ticket/12713

@kini kini added this to the sage-5.0 milestone Mar 20, 2012
@kini

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:2

For what it's worth, I removed the moin spkg and modified spkg/install and spkg/standard/deps in the obvious ways. Sage built and all tests passed.

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member

Author: John Palmieri

@jhpalmieri
Copy link
Member

comment:3

Here's a patch. If we go through with this, should we keep MoinMoin as an optional spkg? Probably we should.

@jhpalmieri
Copy link
Member

root repo

@jhpalmieri
Copy link
Member

comment:4

Attachment: trac_12713-moin.patch.gz

@jhpalmieri
Copy link
Member

Dependencies: #10492

@nexttime
Copy link
Mannequin

nexttime mannequin commented Mar 23, 2012

comment:5

Replying to @jhpalmieri:

Here's a patch. If we go through with this, should we keep MoinMoin as an optional spkg? Probably we should.

Yes! (Make it an optional spkg.) Incidentally, that's what I was thinking of yesterday as well, and a couple of times before...

@jdemeyer
Copy link

comment:6

I guess this is something which should be discussed properly on sage-devel first...

@jhpalmieri

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Apr 2, 2012

comment:8

There's slightly more to do: remove the sage -wiki command line option and remove some files related to Moin Moin Wiki in the Sage library (sorry, I haven't checked which files, but I'm sure there is something).

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:9

Here are new patches.

@jhpalmieri
Copy link
Member

root repo

@jhpalmieri
Copy link
Member

Attachment: trac_12713-root.patch.gz

scripts repo

@jhpalmieri
Copy link
Member

comment:10

Attachment: trac_12713-scripts.patch.gz

The Sage library patch conflicts somewhat with #11409. If #11409 is merged first, then the Sage library patch here may be ignored. If this is merged first, then #11409 will need rebasing.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 3, 2012

comment:11

Replying to @jdemeyer:

There's slightly more to do: remove the sage -wiki command line option and remove some files related to Moin Moin Wiki in the Sage library (sorry, I haven't checked which files, but I'm sure there is something).

Should we really remove all traces of MoinMoin (or any kind of wiki) from Sage?

If we keep it as an optional (or experimental) spkg, we could treat it as such, just like the others. I.e., just add checks whether it is present / installed, and give messages according to the result.

@kini
Copy link
Contributor Author

kini commented Apr 3, 2012

comment:12

For an optional spkg, maybe, but do we really maintain (or need to maintain) library/interface support for experimental packages?

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 3, 2012

comment:13

Replying to @kini:

For an optional spkg, maybe, but do we really maintain (or need to maintain) library/interface support for experimental packages?

Well, I don't have strong feelings about that, but probably there are Sage users out there who really used that stuff... :-)

@jhpalmieri
Copy link
Member

comment:14

Since no one on sage-devel objected to the removal of MoinMoin, I think we can do this. If someone wants to restore the functionality, they can just restore the files from sage/server/wiki: that's the whole purpose of having a repository, right? The only other possibility, I think, is to not only add checks for the presence of the package, but also to add doctests to the file sage/server/wiki/moin.py. Removing the files is by far the path of least resistance. Oh, Leif, unless you're volunteering...

@jdemeyer
Copy link

jdemeyer commented Apr 3, 2012

comment:15

Replying to @jhpalmieri:

The Sage library patch conflicts somewhat with #11409. If #11409 is merged first, then the Sage library patch here may be ignored. If this is merged first, then #11409 will need rebasing.

It is unlikely that #11409 will get merged any time soon (unfortunately).

@jdemeyer
Copy link

jdemeyer commented Apr 4, 2012

Changed dependencies from #10492 to #10492, #12515

@jdemeyer
Copy link

jdemeyer commented Apr 4, 2012

comment:17

The file

doc/en/reference/networking.rst

should also be removed, and index.rst modified accordingly.

@jhpalmieri
Copy link
Member

comment:18

Okay, here's a new patch which does that.

@jhpalmieri
Copy link
Member

Sage library

@jhpalmieri
Copy link
Member

comment:19

Attachment: trac_12713-sage.patch.gz

Do we also need a patch for sagenb, removing sagenb/notebook/wiki2html.py? Here is one.

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member

sagenb

@jdemeyer
Copy link

jdemeyer commented Apr 6, 2012

comment:20

Attachment: trac_12713-sagenb.patch.gz

You forgot this (sage library):

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -1008,7 +1008,6 @@
                      'sage.server.simple',
                      'sage.server.notebook',
                      'sage.server.notebook.compress',
-                     'sage.server.wiki',
                      'sage.server.trac',

                      'sage.structure',

@jdemeyer
Copy link

jdemeyer commented Apr 6, 2012

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link

jdemeyer commented Apr 6, 2012

comment:21

positive_review if setup.py is fixed.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Apr 7, 2012

comment:22

Attachment: 12713-sage-review.patch.gz

@jhpalmieri
Copy link
Member

comment:23

Thank you for taking care of that last piece.

@jdemeyer
Copy link

jdemeyer commented Apr 8, 2012

Merged: sage-5.0.beta13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants