ECP-VeloC / AXL

Asynchronous Transfer Library
MIT License
2 stars 8 forks source link

Setup Travis CI #29

Closed CamStan closed 5 years ago

CamStan commented 5 years ago

This sets up Travis CI testing and builds on multiple compilers that are on our machines. Can always scale back if it's overkill.

This will allow PR #4 to be closed since @becker33 has abandoned us.

tonyhutter commented 5 years ago

I love the multiple compiler targets! Awesome!

  1. Do you need -DMPI=ON?: https://travis-ci.com/ECP-VeloC/AXL/jobs/187585322#L795

  2. Can you have the builds fail on warnings?: https://travis-ci.com/ECP-VeloC/AXL/jobs/187585322#L875

CamStan commented 5 years ago

I've never had CI set up to fail on warnings, but it might be possible if we set the -Werror C_FLAG. I'd have to test that if that's the route we want to go.

tonyhutter commented 5 years ago

My vote would be for -Wall -Werror

adammoody commented 5 years ago

I don't think AXL has an MPI option, since it doesn't use MPI. Also, we probably don't need to enable MPI in the kvtree build for AXL either if you want to leave that out.

CamStan commented 5 years ago

Pulled out MPI since AXL doesn't use it and KVTree's Travis already tests with it.

I added -Wall -Werror and started cleaning up the errors to get it to pass, but ran into an issue here https://travis-ci.org/ECP-VeloC/AXL/jobs/511226558#L723 where Travis complains the function is unused.

Essentially, axl_async_daemon() only gets called by other functions inside of #ifdef HAVE_DAEMON blocks which in this case is turned off.

https://github.com/ECP-VeloC/AXL/blob/d427c238d0f24dd874a7deb852f219dc79695376/src/axl_async_daemon.c#L335-L337

Any thoughts?

CamStan commented 5 years ago

Or would it be better to leave -Wall -Werror out for now so that we can get Travis at least building all PR's? Then we can fix those warnings in other PR's and then put those flags back in.

tonyhutter commented 5 years ago

Or would it be better to leave -Wall -Werror out for now so that we can get Travis at least building all PR's?

I'm fine with that

CamStan commented 5 years ago

Issue #31 addresses the warnings for turning on -Werror.

Otherwise this should be good to go now.

adammoody commented 5 years ago

Nice! Thanks @CamStan and @tonyhutter .