Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#14393 closed task (fixed)

Clean SAGE_ROOT from module_list.py (again)

Reported by: fbissey Owned by: GeorgSWeber
Priority: major Milestone: sage-5.9
Component: build Keywords:
Cc: ohanar, cschwan Merged in: sage-5.9.beta5
Authors: François Bissey Reviewers: Christopher Schwan
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by fbissey)

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:

Attachments (1)

trac14393-cleanup.patch (27.5 KB) - added by jdemeyer 6 years ago.
clean up module_list.py

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by fbissey

  • Description modified (diff)
  • Status changed from new to needs_review

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

comment:2 Changed 6 years ago by fbissey

  • Cc cschwan added

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.

comment:3 Changed 6 years ago by cschwan

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

comment:4 follow-up: Changed 6 years ago by fbissey

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.

comment:5 Changed 6 years ago by cschwan

OK, I will test it this weekend.

comment:6 Changed 6 years ago by cschwan

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

comment:7 Changed 6 years ago by fbissey

  • Authors set to Francois Bissey
  • Reviewers set to Christopher Schwan
  • Status changed from needs_review to positive_review

OK I put it in positive review then.

Changed 6 years ago by jdemeyer

clean up module_list.py

comment:8 Changed 6 years ago by jdemeyer

  • Authors changed from Francois Bissey to François Bissey

comment:9 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.9.beta5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:10 in reply to: ↑ 4 Changed 6 years ago by leif

Replying to fbissey:

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.

Note: See TracTickets for help on using tickets.