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.

Sign up for GitHub

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

Jump to bottom

Test appveyor.yml #588

Closed
wants to merge 3 commits into from
Closed

Test appveyor.yml #588

wants to merge 3 commits into from

Conversation

cloudhan
Copy link
Contributor

writing script is a bit tricky, now escape char "#" as "#"

@cloudhan
Update .appveyor.yml
b3bd09a 
explicitly define ARMA_64BIT_WORD for _Win64 and ARMA_32BIT_WORD otherwise

It potentially eliminate tons of warning `conversion from 'size_t' to 'const arma::uword', possible loss of data`
@cloudhan
Update .appveyor.yml
5cbcc05 
escape comment char
@rcurtin
Copy link
Member

rcurtin commented Mar 22, 2016

We should do this in src/mlpack/core/arma_extend/arma_extend.hpp instead of in the AppVeyor configuration. You can use the different macros that Windows defines to detect the operating system and compiler. (I think they are called _WIN32, _WIN64, _MSC_VER, and similar. Not exactly sure how they need to be used here, but it should not be too hard.)

@cloudhan
Copy link
Contributor Author

I was trying to replace the line // #define ARMA_64BIT_WORD with

#ifdef _WIN64
#define ARMA_64BIT_WORD
#else
#define ARMA_32BIT_WORD
#endif

but it seems don't work on AppVeyor

@cloudhan
Update arma_extend.hpp
7774786 
Manually set ARMA_{64,32}BIT_WORD for _WIN64 or win32 to eliminate `warning C4267` for windows build
@cloudhan cloudhan closed this Mar 22, 2016
@cloudhan cloudhan deleted the patch-1 branch March 22, 2016 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@cloudhan @rcurtin