Opened 2 years ago

Closed 2 years ago

#25835 closed enhancement (fixed)

Move gap packages to features

Reported by: gh-timokau Owned by:
Priority: major Milestone: sage-8.4
Component: build Keywords:
Cc: tscrim, jdemeyer Merged in:
Authors: Timo Kaufmann Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 63363f0 (Commits) Commit: 63363f02bb5ef2d12791341abfc36fa0f8221128
Dependencies: Stopgaps:


Split from #25825. This moves the remaining instances of is_package_installed for gap packages to Features.

Change History (7)

comment:1 Changed 2 years ago by gh-timokau

  • Status changed from new to needs_review

comment:2 Changed 2 years ago by gh-timokau

  • Milestone changed from sage-8.3 to sage-8.4

comment:3 Changed 2 years ago by tscrim

You will also want to include #22623 (looking at this ticket, I think I did a poor job of actually testing if GAP could load the QuaGroup package on #22623 as I just let it fail).

comment:4 Changed 2 years ago by gh-timokau

  • Status changed from needs_review to needs_work

You probably know your code best. Could you maybe make that addition? Feel free to base off my branch and rebase as you like.

comment:5 Changed 2 years ago by tscrim

  • Branch changed from u/gh-timokau/gap-features to u/tscrim/gap_features-25835
  • Commit changed from 34aebef467b36909f5e2af2c928fb93e31f2d363 to 63363f02bb5ef2d12791341abfc36fa0f8221128
  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_work to needs_review

Let me know if I did this correctly. If so, then you can set this to a positive review.

New commits:

edc7141Merge branch 'u/gh-timokau/gap-features' of git:// into u/tscrim/gap_features-25835
63363f0Adding GapPackage to

comment:6 Changed 2 years ago by gh-timokau

  • Status changed from needs_review to positive_review

LGTM (I'm assuming you ran the doctest?) :)

Just as an FYI: My one nitpick would be the commit message. It should be in imperative and a bit more abstract, maybe "Use features to detect QuaGroup support" or something. But that's not a blocker.

comment:7 Changed 2 years ago by vbraun

  • Branch changed from u/tscrim/gap_features-25835 to 63363f02bb5ef2d12791341abfc36fa0f8221128
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.