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

Clean SAGE_ROOT from module_list.py (again) #14393

Closed
kiwifb opened this issue Mar 31, 2013 · 14 comments
Closed

Clean SAGE_ROOT from module_list.py (again) #14393

kiwifb opened this issue Mar 31, 2013 · 14 comments
Assignees
Milestone

Comments

@kiwifb
Copy link
Member

kiwifb commented Mar 31, 2013

A few tickets have been using SAGE_ROOT directly or not used SAGE_INC since my last clean up. After cleaning direct calls to SAGE_ROOT in the rest of sage we need to spend some time here.

The use of SAGE_INC has also been normalized everywhere so it is not necessary to add a final "/". The use of predefined depend variables has been enforced as much as I could see it.

Apply to the sage library:

CC: @ohanar @cschwan

Component: build

Author: François Bissey

Reviewer: Christopher Schwan

Merged: sage-5.9.beta5

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

@kiwifb
Copy link
Member Author

kiwifb commented Mar 31, 2013

comment:1

It has been bothering me for a while that a few people have been using naked SAGE_ROOT in there.

@kiwifb

This comment has been minimized.

@kiwifb
Copy link
Member Author

kiwifb commented Apr 5, 2013

comment:2

Could you have a look at this Christopher? I was hoping Andrew would be interested but he must be busy elsewhere. I'd very much want to see this in 5.9.

@cschwan
Copy link
Mannequin

cschwan mannequin commented Apr 5, 2013

comment:3

Looks good to me. Do you want me to test it as well? I could do this at the weekend.

@kiwifb
Copy link
Member Author

kiwifb commented Apr 5, 2013

comment:4

It should be done properly, so yes testing. Testing is the only way to see if I missed something, which I don't think I did.

@cschwan
Copy link
Mannequin

cschwan mannequin commented Apr 5, 2013

comment:5

OK, I will test it this weekend.

@cschwan
Copy link
Mannequin

cschwan mannequin commented Apr 7, 2013

comment:6

I checked with beta2; patch applies cleanly and the build is fine.

@kiwifb
Copy link
Member Author

kiwifb commented Apr 8, 2013

Reviewer: Christopher Schwan

@kiwifb
Copy link
Member Author

kiwifb commented Apr 8, 2013

comment:7

OK I put it in positive review then.

@kiwifb
Copy link
Member Author

kiwifb commented Apr 8, 2013

Author: Francois Bissey

@jdemeyer
Copy link

jdemeyer commented Apr 9, 2013

Attachment: trac14393-cleanup.patch.gz

clean up module_list.py

@jdemeyer
Copy link

Changed author from Francois Bissey to François Bissey

@jdemeyer
Copy link

Merged: sage-5.9.beta5

@nexttime
Copy link
Mannequin

nexttime mannequin commented May 4, 2013

comment:10

Replying to @kiwifb:

It should be done properly, so yes testing. Testing is the only way to see if I missed something, which I don't think I did.

You only missed one instance (#14531), for an optional spkg.

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

2 participants