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

Msvc compliance #2

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Msvc compliance #2

wants to merge 7 commits into from

Conversation

Bentoy13
Copy link

@Bentoy13 Bentoy13 commented Oct 9, 2019

Minor modifications in the code to be compliant with VS2017. Compilation runs well but not currently tested on samples.
Also, I am working on CMake files to generate the project files.

@Bentoy13
Copy link
Author

Bentoy13 commented Oct 9, 2019

I have done a draft for CMake. Please note that I am not mastering CMake, so it may be certainly improved. I think it would be nice to be tested on Linux to see if Makefile is generated properly. I have tried to isolate some options proper to VS.

@Bentoy13 Bentoy13 marked this pull request as ready for review October 9, 2019 14:20
@Bentoy13
Copy link
Author

Bentoy13 commented Oct 9, 2019

Working on provided examples.

Copy link
Collaborator

@newling newling left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution, and sorry for the slow response.

Can you please separate into 2 PRs:
(1) CMake support (files CMakeLists.txt, src/CMakeLists.txt, src/arrutilv2l0.h, src/initialise2.h)
(2) headers and the use of &&.

I will definitely accept (2) immediately, but will need to consider (1) in more detail.

Thanks!

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