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

Set a default external enumeration library #456

Merged
merged 8 commits into from
Nov 26, 2020
Merged

Set a default external enumeration library #456

merged 8 commits into from
Nov 26, 2020

Conversation

cr-marcstevens
Copy link
Collaborator

This PR makes it possible to directly integrate any external enumeration into fplll and be used by default by fplll.

@malb
Copy link
Collaborator

malb commented Nov 24, 2020

Looks good to me, so once the checkstyle issue is fixed, happy to merge.

@cr-marcstevens
Copy link
Collaborator Author

Thanks, but indeed all checks failed, so have to fix style and maybe other things.

@cr-marcstevens
Copy link
Collaborator Author

BTW this pull request moves the extenum API from the fplll namespace to the main namespace ::.
This is because the header file doesn't use any other fplll header files, so that it can be copied to extenum libraries without dependencies.

What are your thoughts on this:

  1. Should this be moved back to fplll?
    1a. If yes, do we use macros then or just hardcoded fplll as namespace?

@malb
Copy link
Collaborator

malb commented Nov 24, 2020

TBH, I don't have a strong opinion either way. What's the risk of name collision in some bigger project, i.e. should it be in some namespace?

@codecov
Copy link

codecov bot commented Nov 24, 2020

Codecov Report

Merging #456 (da91584) into master (1ac8c38) will decrease coverage by 0.00%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #456      +/-   ##
==========================================
- Coverage   64.74%   64.73%   -0.01%     
==========================================
  Files          75       75              
  Lines        7432     7433       +1     
==========================================
  Hits         4812     4812              
- Misses       2620     2621       +1     
Impacted Files Coverage Δ
fplll/enum-parallel/enumlib.cpp 100.00% <ø> (ø)
fplll/enum/enumerate_base.h 100.00% <ø> (ø)
fplll/enum/enumerate_ext.cpp 77.19% <ø> (ø)
fplll/enum/enumerate_ext.h 44.44% <50.00%> (-5.56%) ⬇️
fplll/enum-parallel/enumlib_dim.cpp 76.92% <66.66%> (ø)
fplll/enum/enumerate.h 79.31% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ac8c38...da91584. Read the comment docs.

@cr-marcstevens
Copy link
Collaborator Author

@malb do you want to push the green button?

@malb malb merged commit c3533db into fplll:master Nov 26, 2020
@malb
Copy link
Collaborator

malb commented Nov 26, 2020

With pleasure.

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