Iox #590 Rename utils to hoofs #790

mossmaurice · 2021-05-11T21:16:06Z

Pre-Review Checklist for the PR Author

Notes for Reviewer

Rename iceoryx_utils to iceoryx_hoofs

My apologies for the large pull request 🐿️ I tried smaller steps, but eventually decided to do it in one go.

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

All open points are addressed and tracked via issues

References

Closes partly Improve modularity by creating architecure guidelines #590
Closes Enforce unix line endings #794

codecov · 2021-05-11T21:45:53Z

Codecov Report

Merging #790 (c121a8c) into master (db2db48) will increase coverage by 0.09%.
The diff coverage is 96.61%.

❗ Current head c121a8c differs from pull request most recent head b56bac4. Consider uploading reports for the commit b56bac4 to get more accurate results

@@            Coverage Diff             @@
##           master     #790      +/-   ##
==========================================
+ Coverage   74.24%   74.34%   +0.09%     
==========================================
  Files         322      322              
  Lines       11520    11522       +2     
  Branches     1952     1952              
==========================================
+ Hits         8553     8566      +13     
+ Misses       2206     2195      -11     
  Partials      761      761

Flag	Coverage Δ
unittests	`73.17% <96.61%> (+0.08%)`	⬆️
unittests_timing	`30.74% <5.08%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ceoryx_binding_c/source/c2cpp_enum_translation.cpp	`46.66% <ø> (ø)`
iceoryx_binding_c/source/c_node.cpp	`100.00% <ø> (ø)`
iceoryx_binding_c/source/c_notification_info.cpp	`100.00% <ø> (ø)`
iceoryx_binding_c/source/cpp2c_publisher.cpp	`16.66% <ø> (ø)`
...c/source/cpp2c_service_description_translation.cpp	`100.00% <ø> (ø)`
iceoryx_binding_c/source/cpp2c_subscriber.cpp	`78.94% <ø> (ø)`
...ceoryx_dds/include/iceoryx_dds/dds/data_reader.hpp	`100.00% <ø> (ø)`
...nclude/iceoryx_dds/internal/gateway/dds_to_iox.inl	`44.18% <ø> (ø)`
iceoryx_dds/source/dds2iceoryx_app/main.cpp	`0.00% <ø> (ø)`
iceoryx_dds/source/iceoryx2dds_app/main.cpp	`0.00% <ø> (ø)`
... and 159 more

iceoryx_examples/iceperf/iceperf_laurel.cpp

elBoberido

First 284 files. The remaining ones will follow

README.md

elBoberido · 2021-05-12T10:11:05Z

iceoryx_hoofs/README.md


-The iceoryx utils are our basic building blocks - the foundation of
+The iceoryx hoofs (**H**andy **O**bjects **O**ptimised **F**or **S**afety) are our basic building blocks - the foundation of


Is this acronym correct? I read a different one in one of the previous files ... don't know which one, there are just too many

Jep, see here

can we find an acronym where H stands for hypnotoad?

I think this would be blasphemy, using Hypnotoad as a helper or a name for building blocks ;). We all praise Hypnotoad and work for it and not the other way around

iceoryx_hoofs/include/iceoryx_hoofs/concurrent/lockfree_queue.hpp

elBoberido · 2021-05-12T10:32:51Z

iceoryx_hoofs/platform/linux/include/iceoryx_hoofs/platform/acl.hpp

+//
+// SPDX-License-Identifier: Apache-2.0
+#ifndef IOX_HOOFS_LINUX_PLATFORM_ACL_HPP
+#define IOX_HOOFS_LINUX_PLATFORM_ACL_HPP


why is this whole file marked as new

TBH, I have no clue :( I've moved all the files with the same git mv command. I know that Git has some magic detection mechanisms to determine how many percent of a file has changed. I don't understand why this worked with the other platform files, though.

Double checked, no duplicates around:

./iceoryx_hoofs/platform/qnx/include/iceoryx_hoofs/platform/acl.hpp ./iceoryx_hoofs/platform/mac/include/iceoryx_hoofs/platform/acl.hpp ./iceoryx_hoofs/platform/linux/include/iceoryx_hoofs/platform/acl.hpp ./iceoryx_hoofs/platform/win/include/iceoryx_hoofs/platform/acl.hpp

Did you call git mv iceoryx_utils iceoryx_hoofs or did you do it for every single file?

I think this is just the github heuristic which is doesn't recognize that it is an old file. I looked at the history of the file and it is preserved

The former. Maybe I did something wrong when adding the files.

iceoryx_hoofs/platform/linux/include/iceoryx_hoofs/platform/dlfcn.hpp

iceoryx_hoofs/platform/linux/include/iceoryx_hoofs/platform/getopt.hpp

elBoberido

400 files done and I just noticed you didn't run clang-format

elfenpiff · 2021-05-14T09:00:38Z

iceoryx_hoofs/README.md


-The iceoryx utils are our basic building blocks - the foundation of
+The iceoryx hoofs (**H**andy **O**bjects **O**ptimised **F**or **S**afety) are our basic building blocks - the foundation of


I think this would be blasphemy, using Hypnotoad as a helper or a name for building blocks ;). We all praise Hypnotoad and work for it and not the other way around

iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/list.inl

elfenpiff · 2021-05-14T09:43:37Z

iceoryx_hoofs/platform/linux/include/iceoryx_hoofs/platform/acl.hpp

+//
+// SPDX-License-Identifier: Apache-2.0
+#ifndef IOX_HOOFS_LINUX_PLATFORM_ACL_HPP
+#define IOX_HOOFS_LINUX_PLATFORM_ACL_HPP


Did you call git mv iceoryx_utils iceoryx_hoofs or did you do it for every single file?

elBoberido

600 filed done ... time for lunch

iceoryx_hoofs/source/log/logging_internal.cpp

elBoberido

done

iceoryx_integrationtest/Readme.md

elfenpiff · 2021-05-14T13:16:50Z

iceoryx_hoofs/platform/qnx/include/iceoryx_hoofs/platform/un.hpp

@@ -1,4 +1,5 @@
 // Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved.
+// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.


This is weird? Why did you move wait.hpp to un.hpp?
Where is the original wait.hpp file?! Or did here something else go wrong ... I am a little bit lost

That's odd, indeed. Shoulnd't be an issue since wait.hpp and un.hpp are definitely there. I have the feeling I need to practice this dance more thoroughly.

elfenpiff

Could you please replace #pragma once with include guards in iceoryx_hoofs/platform/win/include/iceoryx_hoofs/platform/platform_correction.hpp

elfenpiff · 2021-05-14T14:11:54Z

tools/HelixQAC/README.md

@@ -20,7 +20,7 @@ with the compiler command being a command used to build an iceoryx file. If you

 an example command if you use GCC (command taked from the build log):

-    CCT_Generator.sh -c /usr/bin/c++  -Diceoryx_utils_EXPORTS -I/home/perforce/iceoryx/iceoryx_utils/include -I/home/perforce/iceoryx/iceoryx_utils/platform/linux/include  -O3 -DNDEBUG -fPIC   -W -Wall -Wextra -Wuninitialized -Wpedantic -Wstrict-aliasing -Wcast-align -Wno-noexcept-type -Wconversion -std=gnu++14 -o CMakeFiles/iceoryx_utils.dir/source/posix_wrapper/unix_domain_socket.cpp.o -c /home/perforce/iceoryx/iceoryx_utils/source/posix_wrapper/unix_domain_socket.cpp
+    CCT_Generator.sh -c /usr/bin/c++  -Diceoryx_hoofs_EXPORTS -I/home/perforce/iceoryx/iceoryx_hoofs/include -I/home/perforce/iceoryx/iceoryx_hoofs/platform/linux/include  -O3 -DNDEBUG -fPIC   -W -Wall -Wextra -Wuninitialized -Wpedantic -Wstrict-aliasing -Wcast-align -Wno-noexcept-type -Wconversion -std=gnu++14 -o CMakeFiles/iceoryx_hoofs.dir/source/posix_wrapper/unix_domain_socket.cpp.o -c /home/perforce/iceoryx/iceoryx_hoofs/source/posix_wrapper/unix_domain_socket.cpp


Does this actually work, is this somewhere tested? I mean where is this -Diceoryx_hoofs_EXPORTS defined since I do not find the corresponding CCT_Generator.sh script in our repo I assume the script uses something from us where we define that compiler variable?!

It was tested in #757 by @dkroenke @marthtz and @toniglandy1. IIUC these are compiler switches from CMake.

@elfenpiff
-D<libname>_EXPORTS is automatically generated by cmake when building a shared library.

From https://cmake.org/cmake/help/v3.0/command/set_target_properties.html:

DEFINE_SYMBOL sets the name of the preprocessor symbol defined when compiling sources in a shared library. If not set here then it is set to target_EXPORTS by default (with some substitutions if the target is not a valid C identifier). This is useful for headers to know whether they are being included from inside their library or outside to properly setup dllexport/dllimport decorations.

Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

…g to dependencies Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

…ng_c} Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

…f origin/master and introspection files Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

Signed-off-by: Mathias Kraus <mathias.kraus@apex.ai>

…l and remove left-over pragma once Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

mossmaurice added the refactoring Refactor code without adding features label May 11, 2021

mossmaurice requested review from dkroenke and elBoberido May 11, 2021 21:16

mossmaurice self-assigned this May 11, 2021

elfenpiff reviewed May 12, 2021

View reviewed changes

iceoryx_examples/iceperf/iceperf_laurel.cpp Outdated Show resolved Hide resolved

mossmaurice force-pushed the iox-#590-rename-utils-to-hoofs branch from b22f485 to f7e7f92 Compare May 12, 2021 10:20

mossmaurice requested a review from elfenpiff May 12, 2021 10:24

elfenpiff removed the request for review from dkroenke May 12, 2021 10:24

elBoberido reviewed May 12, 2021

View reviewed changes

mossmaurice force-pushed the iox-#590-rename-utils-to-hoofs branch from c2e0eaf to eb8f165 Compare May 12, 2021 17:00

mossmaurice requested a review from elBoberido May 12, 2021 17:05

elfenpiff reviewed May 14, 2021

View reviewed changes

elBoberido reviewed May 14, 2021

View reviewed changes

iceoryx_hoofs/source/log/logging_internal.cpp Show resolved Hide resolved

elBoberido reviewed May 14, 2021

View reviewed changes

iceoryx_integrationtest/Readme.md Outdated Show resolved Hide resolved

elfenpiff reviewed May 14, 2021

View reviewed changes

mossmaurice and others added 11 commits May 14, 2021 20:30

iox-eclipse-iceoryx#590 Rename iceoryx_utils to iceoryx_hoofs

a29fd40

Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

iox-eclipse-iceoryx#590 Change line ending to LF

4a3d648

Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

iox-eclipse-iceoryx#590 Update copyright in header

1d26163

Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

iox-eclipse-iceoryx#590 Reorder CMake targets in main readme accordin…

eb86655

…g to dependencies Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

iox-eclipse-iceoryx#590 Run clang-format over iceoryx_hoofs

97de725

Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

iox-eclipse-iceoryx#590 Run clang-format over iceoryx_posh

b9c30ab

Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

iox-eclipse-iceoryx#590 Run clang-format over iceoryx_dds

bfe9c74

Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

iox-eclipse-iceoryx#590 Run clang-format over iceoryx_{examples,bindi…

e9b7b8b

…ng_c} Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

iox-eclipse-iceoryx#590 Clang-format touched files since last merge o…

1d76e20

…f origin/master and introspection files Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

iox-eclipse-iceoryx#794 use unix line endings on all files

a7db396

Signed-off-by: Mathias Kraus <mathias.kraus@apex.ai>

iox-eclipse-iceoryx#590 Fix copyright header, make things alphabetica…

b56bac4

…l and remove left-over pragma once Signed-off-by: Simon Hoinkis <simon.hoinkis@apex.ai>

elBoberido force-pushed the iox-#590-rename-utils-to-hoofs branch from c121a8c to b56bac4 Compare May 14, 2021 19:21

elBoberido approved these changes May 14, 2021

View reviewed changes

elfenpiff approved these changes May 14, 2021

View reviewed changes

mossmaurice merged commit 11c18ca into eclipse-iceoryx:master May 14, 2021

dkroenke linked an issue Jun 15, 2021 that may be closed by this pull request

Improve modularity by creating architecure guidelines #590

Open

20 tasks

elBoberido deleted the iox-#590-rename-utils-to-hoofs branch August 2, 2021 16:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Iox #590 Rename utils to hoofs #790

Iox #590 Rename utils to hoofs #790

mossmaurice commented May 11, 2021 •

edited by elfenpiff

Loading

codecov bot commented May 11, 2021 •

edited

Loading

elBoberido left a comment

elBoberido May 12, 2021

mossmaurice May 12, 2021

elBoberido May 12, 2021

elfenpiff May 14, 2021

elBoberido May 12, 2021

mossmaurice May 12, 2021

elfenpiff May 14, 2021

elBoberido May 14, 2021

mossmaurice May 14, 2021

elBoberido left a comment

elfenpiff May 14, 2021

elfenpiff May 14, 2021

elBoberido left a comment

elBoberido left a comment

elfenpiff May 14, 2021

mossmaurice May 14, 2021

elfenpiff left a comment

elfenpiff May 14, 2021

mossmaurice May 14, 2021

dkroenke May 14, 2021


		The iceoryx utils are our basic building blocks - the foundation of
		The iceoryx hoofs (Handy Objects Optimised For Safety) are our basic building blocks - the foundation of

		@@ -1,4 +1,5 @@
		// Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved.
		// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.

Iox #590 Rename utils to hoofs #790

Iox #590 Rename utils to hoofs #790

Conversation

mossmaurice commented May 11, 2021 • edited by elfenpiff Loading

Pre-Review Checklist for the PR Author

Notes for Reviewer

Checklist for the PR Reviewer

Post-review Checklist for the PR Author

References

codecov bot commented May 11, 2021 • edited Loading

Codecov Report

elBoberido left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elBoberido left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elBoberido left a comment

Choose a reason for hiding this comment

elBoberido left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elfenpiff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mossmaurice commented May 11, 2021 •

edited by elfenpiff

Loading

codecov bot commented May 11, 2021 •

edited

Loading