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

Move gap packages to features #25835

Closed
timokau opened this issue Jul 11, 2018 · 8 comments
Closed

Move gap packages to features #25835

timokau opened this issue Jul 11, 2018 · 8 comments

Comments

@timokau
Copy link
Contributor

timokau commented Jul 11, 2018

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

CC: @tscrim @jdemeyer

Component: build

Author: Timo Kaufmann

Branch/Commit: 63363f0

Reviewer: Travis Scrimshaw

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

@timokau timokau added this to the sage-8.3 milestone Jul 11, 2018
@timokau timokau modified the milestones: sage-8.3, sage-8.4 Aug 5, 2018
@tscrim
Copy link
Collaborator

tscrim commented Aug 5, 2018

comment:3

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).

@timokau
Copy link
Contributor Author

timokau commented Aug 6, 2018

comment:4

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

@tscrim
Copy link
Collaborator

tscrim commented Aug 6, 2018

Changed branch from u/gh-timokau/gap-features to u/tscrim/gap_features-25835

@tscrim
Copy link
Collaborator

tscrim commented Aug 6, 2018

comment:5

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://trac.sagemath.org/sage into u/tscrim/gap_features-25835
63363f0Adding GapPackage to quantum_group_gap.py.

@tscrim
Copy link
Collaborator

tscrim commented Aug 6, 2018

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 6, 2018

Changed commit from 34aebef to 63363f0

@timokau
Copy link
Contributor Author

timokau commented Aug 6, 2018

comment:6

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.

@vbraun
Copy link
Member

vbraun commented Aug 7, 2018

Changed branch from u/tscrim/gap_features-25835 to 63363f0

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