Opened 4 years ago

Closed 4 years ago

#24402 closed enhancement (fixed)

Clean up MPFR and MPFI declarations

Reported by: jdemeyer Owned by:
Priority: critical Milestone: sage-8.2
Component: interfaces Keywords:
Cc: embray, tscrim Merged in:
Authors: Jeroen Demeyer Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 1168fe7 (Commits, GitHub, GitLab) Commit: 1168fe76fd037f2e6d54f37860b5c84989cf7722
Dependencies: Stopgaps:

Status badges

Description


Change History (11)

comment:1 Changed 4 years ago by jdemeyer

  • Summary changed from Clean up MPFI declarations to Clean up MPFR and MPFI declarations

comment:2 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/clean_up_mpfi_declarations

comment:3 Changed 4 years ago by jdemeyer

  • Commit set to 311701b9d06009c41fe02313f72af991b3d218b2
  • Status changed from new to needs_review

New commits:

311701bClean up MPFR and MPFI declarations

comment:4 Changed 4 years ago by git

  • Commit changed from 311701b9d06009c41fe02313f72af991b3d218b2 to 1168fe76fd037f2e6d54f37860b5c84989cf7722

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

1168fe7Clean up MPFR and MPFI declarations

comment:5 Changed 4 years ago by jdemeyer

  • Cc embray added

Erik: it would be good to test this on Cygwin.

comment:6 Changed 4 years ago by tscrim

  • Cc tscrim added
  • Reviewers set to Travis Scrimshaw

LGTM, but I agree with Jeroen that we should test this on Cygwin as that can be a bit more fickle about libraries and linking. I will try to get to this this week, but I may not be able to.

comment:7 Changed 4 years ago by embray

The patchbot should get to it soon I imagine. In fact I'm surprised it hasn't already, since it's been running a bit more efficiently lately.

I think I will change its configuration so that tickets with "cygwin" in their keywords can be given higher priority. In the meantime, if you change the ticket's priority (say to critical) it will also be weighted higher.

Last edited 4 years ago by embray (previous) (diff)

comment:8 Changed 4 years ago by embray

If I understand correctly, I think that the strategically placed # distutils: libraries = declarations should be all that's necessary here and lgtm in principle. Will still want to test though. If for some reason the patchbot takes too much longer I'll test it by hand soon.

comment:9 Changed 4 years ago by jdemeyer

  • Priority changed from major to critical

comment:10 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to positive_review

I'm going to set this to positive review. It should work and if it doesn't, it will be easy to fix.

comment:11 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/clean_up_mpfi_declarations to 1168fe76fd037f2e6d54f37860b5c84989cf7722
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.