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

CMake and CTest refactoring #44

Merged
merged 12 commits into from
Mar 14, 2019
Merged

Conversation

cpockrandt
Copy link
Collaborator

@cpockrandt cpockrandt commented Jul 14, 2018

This is a draft adressing some of the changes discussed in #39. I included commits from the open PR #40 for now to avoid errors on Travis.

@h-2 Could you start reviewing this? Feel free to comment on the open TODOs:

  • remove install.sh and include installing header files into cmake (done after removing libdivsufsort)
  • there was something with LLVM in .travis.yml that I didn't really understood (Since osx builds are already built on travis, linking against libc++ is already done. I don't see any need to test this manually)
  • redesigning CMakeLists.txt: should there be a cmake in the root directory or just one each in test/ tutorial/ and eamples/ ? (I think for convenience of users, there should be a root CMakeLists that includes targets from examples and tutorials but no tests)
  • We have to fix some failing debug tests (some assertions are not met, see Travis). Could you look into this @mpetri? I'm not familiar with the code to see whether the test case is wrong or the assertion in the code. It's nothing serious I think, just an edge case in the child() method of compressed suffix trees
  • writing test cases for in-memory usage and remove all other *-im test cases

@cpockrandt cpockrandt changed the title Tests refactoring2 CMake and CTest refactoring Jul 14, 2018
@cpockrandt cpockrandt mentioned this pull request Jul 14, 2018
1 task
@h-2
Copy link

h-2 commented Jul 16, 2018

In general this looks ok to me, but I don't know coding conventions in the SDSL. I would also argue for keeping one clang instance in the CI.
Has there been a decision, yet, to keep/drop support for GCC4.9? I would strongly argue for dropping it, because of general buggy-ness with C++11/14 things, and also because I am preparing a PR for serialisation support through cereal and it is just so much easier to integrate if I can use __has_include which is not available before GCC5. What are your thoughts @mpetri and @simongog ?
Debian stable comes with GCC5 by default and both of the last two Ubuntu long-term-support releases (16.04 and 18.04) have GCC>=5 available, I think it's a fair assumption that that covers almost everybody 😉

@cpockrandt cpockrandt force-pushed the tests_refactoring2 branch from 52e2ed1 to c7c1ab3 Compare July 17, 2018 18:17
@cpockrandt cpockrandt force-pushed the tests_refactoring2 branch 2 times, most recently from f6d60a2 to 6c4742c Compare October 6, 2018 12:15
@cpockrandt cpockrandt force-pushed the tests_refactoring2 branch 4 times, most recently from 8946268 to f739d54 Compare December 26, 2018 00:12
@cpockrandt cpockrandt force-pushed the tests_refactoring2 branch 5 times, most recently from f26ecc4 to 71d7cf9 Compare March 13, 2019 23:03
@cpockrandt cpockrandt force-pushed the tests_refactoring2 branch 2 times, most recently from 3f80586 to f329455 Compare March 14, 2019 02:49
@cpockrandt cpockrandt force-pushed the tests_refactoring2 branch 3 times, most recently from 814fc29 to 3bb14c8 Compare March 14, 2019 10:40
@cpockrandt
Copy link
Collaborator Author

I'm merging this now. Jenkins is set up and running, max. job execution time was increased by Travis.

The failing tests (that were also failing before) will be addressed in new issues and pull requests.

@cpockrandt cpockrandt merged commit bd9ce15 into xxsds:master Mar 14, 2019
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