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

Added Travis file for CI #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added Travis file for CI #26

wants to merge 1 commit into from

Conversation

amallia
Copy link
Contributor

@amallia amallia commented Jun 27, 2017

@mavam for review

Copy link
Owner

@mavam mavam left a comment

Choose a reason for hiding this comment

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

Good idea, CI is long overdue!

@@ -31,6 +31,7 @@ set(CMAKE_CXX_FLAGS_DEBUG "-O0 -g")
set(CMAKE_CXX_FLAGS_MINSIZEREL "-Os -DNDEBUG")
set(CMAKE_CXX_FLAGS_RELEASE "-O3 -DNDEBUG")
set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O2 -g")
set(CMAKE_EXE_LINKER_FLAGS "-pthread")
Copy link
Owner

Choose a reason for hiding this comment

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

Can you try whether this more portable fix works as well?

# Exists already.
find_package(Threads)
if (NOT Threads_FOUND)
  message(FATAL_ERROR "Could not find system threading libraries")
endif ()

# Add this line.
target_link_libraries(libbf ${CMAKE_THREAD_LIBS_INIT})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CMake Error at CMakeLists.txt:44 (target_link_libraries):
  Cannot specify link libraries for target "libbf" which is not built by this
  project.

Copy link
Owner

Choose a reason for hiding this comment

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

I forgot: this line needs go below after add_library. (line 85)

@@ -333,7 +333,14 @@ bool engine::run(bool colorize,
// psv == pipe-separated-values
strvec result;
if (psv != ".*") {
split(result, psv, "|", token_compress_on);
// Quick & dirty fix
Copy link
Owner

Choose a reason for hiding this comment

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

The comment makes only sense in the diff, so I'd either comment the previous line or remove the comment entirely.

// Quick & dirty fix
std::istringstream ss(psv);
std::string token;
while(std::getline(ss, token, ',')) {
Copy link
Owner

Choose a reason for hiding this comment

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

Style:

  • space around keywords
  • omit braces for single-line statements

Ideally we can use this clang-format style: https://github.com/actor-framework/actor-framework/blob/develop/.clang-format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can set up an automatic style validator for PRs.

@amallia amallia force-pushed the travis branch 5 times, most recently from 7ac0e8b to cb897e0 Compare June 28, 2017 21:37
@mavam
Copy link
Owner

mavam commented Jun 30, 2017

I've pushed some lingering local changes to master. You should be able to safely rebase your topic branch.

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.

None yet

2 participants