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

JuliaInterface: move macros to macros.jl #257

Merged
merged 1 commit into from
Jun 17, 2019

Conversation

fingolfin
Copy link
Member

This is a step towards breaking up gap.jl. It'll conflict with PR #249, but I am prepared to resolve that.

@fingolfin fingolfin requested a review from sebasguts June 16, 2019 22:06
@codecov
Copy link

codecov bot commented Jun 16, 2019

Codecov Report

Merging #257 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #257   +/-   ##
=======================================
  Coverage   69.95%   69.95%           
=======================================
  Files          49       50    +1     
  Lines        3258     3258           
=======================================
  Hits         2279     2279           
  Misses        979      979
Impacted Files Coverage Δ
pkg/GAPJulia/JuliaInterface/julia/gap.jl 46% <ø> (-2.08%) ⬇️
pkg/GAPJulia/JuliaInterface/julia/macros.jl 100% <100%> (ø)

directly inside GAP. The second example returns the function `SymmetricGroup`
via `@gap(SymmetricGroup)`, then calls that function with the argument `3`.
"""
macro gap(str)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I am not even sure we should keep this macro.

Its functionality is very limited, and it is not exactly clear to the user what works and what does not work. So it might be easier to just remove it.

Although, if you have some experience, it leads to nicer and shorter code, specially in presentations. So I am not completely sure what to do with it . . .

@sebasguts sebasguts merged commit a3b3262 into oscar-system:master Jun 17, 2019
@fingolfin fingolfin deleted the mh/macros branch June 17, 2019 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants