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

Iox #433 rework build and test tooling #452

Merged
merged 71 commits into from
Dec 22, 2020
Merged

Iox #433 rework build and test tooling #452

merged 71 commits into from
Dec 22, 2020

Conversation

dkroenke
Copy link
Member

@dkroenke dkroenke commented Dec 19, 2020 •
edited
Loading

Pre-Review Checklist for the PR Author

  1. Branch follows the naming format (iox-#123-this-is-a-branch)
  2. Commits messages are according to this guideline
    • Commit messages have the issue ID (iox-#123 commit text)
    • Commit messages are signed (git commit -s)
    • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
  3. Update the PR title
    • Follow the same conventions as for commit messages
    • Link to the relevant issue
  4. Relevant issues are linked
  5. Add sensible notes for the reviewer
  6. All checks have passed
  7. Assign PR to reviewer

Notes for Reviewer

We have inconsistencies between the cmake build and the build script and documentation, this should be cleaned up.

  • buildflags in iceoryx_meta and build script
  • documentation
  • copyright headers in cmake files
  • add -O2 optimization for gcc/clang
  • harmonize buildflags in cmake and script
  • create a separate cmake file for all options where the user have an overview
  • move more build functionality from bash into cmake
  • test execution can be done in cmake (see tests.cmake in iceoryx_meta)
  • out-of-tree build for all examples

Checklist for the PR Reviewer

  • Commits are properly organized and messages are according to the guideline
  • Code according to our coding style and naming conventions
  • Unit tests have been written for new behavior
  • Public API changes are documented via doxygen
  • Copyright owner are updated in the changed files
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

Post-review Checklist for the Eclipse Committer

  1. All checkboxes in the PR checklist are checked or crossed out
  2. Merge

References

dkroenke and others added 29 commits December 16, 2020 14:47
@dkroenke
iox-eclipse-iceoryx#433 update version number
4f2193b 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 cmake options in an own file
6ce9f98 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
Merge branch 'master' into iox-eclipse-iceoryx#433-rework-build-test-…
d272b88 
…tooling
@dkroenke
iox-eclipse-iceoryx#433 set correct version number
65ac167
@dkroenke
iox-eclipse-iceoryx#433 extend dds docu
0e80641
@dkroenke
iox-eclipse-iceoryx#433 remove zero from iceoryx
79f17ac 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 remove componenttests and create test targets
1dc0508 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 rework build flags
5beeaed 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 remove python script
7aebe58 
Signed-off-by: Dietrich Krönke <[email protected]>
@FerdinandSpitzschnueffler
iox-eclipse-iceoryx#433 update Dockerfile and readme
46edd87 
Signed-off-by: Marika Lehmann <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 out-of-tree build
af263f2 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
Merge branch 'iox-eclipse-iceoryx#433-rework-build-test-tooling' of h…
e821d5f 
…ttps://github.com/ApexAI/iceoryx into iox-eclipse-iceoryx#433-rework-build-test-tooling
@dkroenke
iox-eclipse-iceoryx#433 adapt Github workflows
d67892d 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 harmonize job names in github
28804e2 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
Merge branch 'master' into iox-eclipse-iceoryx#433-rework-build-test-…
8bf5525 
…tooling
@dkroenke
iox-eclipse-iceoryx#433 add clang-tidy build option
4471ad2 
Signed-off-by: Dietrich Krönke <[email protected]>

iox-eclipse-iceoryx#433 lsan not in Github CI

Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 compile_command.json in cmake
8bb1bcf 
Signed-off-by: Dietrich Krönke <[email protected]>

iox-eclipse-iceoryx#433 disable lsan on CI

Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 add test docu
d341ed9 
Signed-off-by: Dietrich Krönke <[email protected]>

iox-eclipse-iceoryx#433 asan only for Github CI

Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 add sanitizer flags
b700b45 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 set optimization to -O2
ccdb596 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 clean up compile warnings
6343d13 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 rework coverage scan
18e9baf 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 add optimizations to tests
f34deaf 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 add copyright header to cmake files
b7a036b 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
Merge branch 'master' into iox-eclipse-iceoryx#433-rework-build-test-…
63ad081 
…tooling
@dkroenke
iox-eclipse-iceoryx#433 adapt out-of-tree build
331c2d8 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 remove pragma for GCC-9
55c2b50 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 fixups
83b3d7d 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 asan scan fixed
b1c151c 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke dkroenke added the tooling All iceoryx related tooling (scripts etc.) label Dec 19, 2020
mossmaurice
mossmaurice requested changes Dec 22, 2020
View reviewed changes
iceoryx_binding_c/CMakeLists.txt Outdated Show resolved Hide resolved
iceoryx_dds/CMakeLists.txt Outdated Show resolved Hide resolved
iceoryx_meta/CMakeLists.txt Outdated Show resolved Hide resolved
iceoryx_utils/platform/mac/include/iceoryx_utils/platform/semaphore.hpp Outdated Show resolved Hide resolved
mossmaurice
mossmaurice previously approved these changes Dec 22, 2020
View reviewed changes
doc/website/getting-started/installation.md
In Integrationtests are multiple classes within one component (e.g. iceoryx_posh) tested together.
The sourcecode of the tests is placed into the folder `test` within the different iceoryx components. You can find there at least a folder `moduletests` and sometimes ``integrationtests`.

when you now want to create a new test you can place the sourcefile directly into the right folder. Cmake will automatically detect the new file when doing a clean build and will add it to a executable. There is no need to add a gtest main function because we already provide it.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be Now, when you want.. better?

elBoberido
elBoberido reviewed Dec 22, 2020
View reviewed changes
tools/docker/Dockerfile Show resolved Hide resolved
tools/iceoryx_build_test.sh Outdated Show resolved Hide resolved
tools/iceoryx_build_test.sh Outdated Show resolved Hide resolved
dkroenke and others added 7 commits December 22, 2020 12:28
@dkroenke
iox-eclipse-iceoryx#433 cleanup buildflags
06ddb03 
Signed-off-by: Dietrich Krönke <[email protected]>
@elBoberido
iox-eclipse-iceoryx#433 Remove Trigger_test from asan blacklist
bfed334 
Signed-off-by: Mathias Kraus <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 readme fixup
7e78521 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
Merge branch 'iox-eclipse-iceoryx#433-rework-build-test-tooling' of h…
3dd0643 
…ttps://github.com/ApexAI/iceoryx into iox-eclipse-iceoryx#433-rework-build-test-tooling
@elBoberido
iox-eclipse-iceoryx#433 Fix Trigger_test and adjust Trigger move assi…
e40170f 
…gnment

Signed-off-by: Mathias Kraus <[email protected]>
@dkroenke
iox-eclipse-iceoryx#433 add flags to examples
af57383 
Signed-off-by: Dietrich Krönke <[email protected]>
@dkroenke
Merge branch 'iox-eclipse-iceoryx#433-rework-build-test-tooling' of h…
48f7ce0 
…ttps://github.com/ApexAI/iceoryx into iox-eclipse-iceoryx#433-rework-build-test-tooling
elBoberido
elBoberido reviewed Dec 22, 2020
View reviewed changes
iceoryx_posh/source/popo/trigger.cpp
@@ -54,6 +54,7 @@ const EventInfo& Trigger::getEventInfo() const noexcept
void Trigger::invalidate() noexcept
{
m_hasTriggeredCallback = cxx::ConstMethodCallback<bool>();
m_resetCallback = cxx::MethodCallback<void, uint64_t>();
Copy link
Member

Choose a reason for hiding this comment

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

@budrus @elfenpiff can you confirm that this is correct?

@dkroenke dkroenke dismissed elfenpiff’s stale review December 22, 2020 13:09

I will follow up in the next tooling PR and discuss with you iceoryx_meta and the CI

@dkroenke dkroenke merged commit 56e2f18 into eclipse-iceoryx:master Dec 22, 2020
@dkroenke dkroenke deleted the iox-#433-rework-build-test-tooling branch December 22, 2020 13:10
@dkroenke dkroenke mentioned this pull request Dec 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elBoberido elBoberido elBoberido approved these changes

@mossmaurice mossmaurice mossmaurice approved these changes

@marthtz marthtz Awaiting requested review from marthtz

@elfenpiff elfenpiff Awaiting requested review from elfenpiff

Assignees

@dkroenke dkroenke

Labels
tooling All iceoryx related tooling (scripts etc.)
Projects
None yet
Milestone
High prio
Development

Successfully merging this pull request may close these issues.

Rework build and test steps in iceoryx
5 participants
@dkroenke @mossmaurice @elfenpiff @elBoberido @FerdinandSpitzschnueffler