Moved rascsi/rasctl specific classes to sub-folders, cleaned up code, fixed SonarCloud issues#889
Moved rascsi/rasctl specific classes to sub-folders, cleaned up code, fixed SonarCloud issues#889
Conversation
This reverts commit 1b28afb.
Merge branch 'develop' into feature_sub_folders
|
SonarCloud Quality Gate failed. |
|
|
||
| $(BINDIR)/$(RASCSI): $(SRC_PROTOBUF) $(OBJ_RASCSI_CORE) $(OBJ_RASCSI) | $(BINDIR) | ||
| $(CXX) $(CXXFLAGS) -o $@ $(OBJ_RASCSI_CORE) $(OBJ_RASCSI) -lpthread -lpcap -lprotobuf -lstdc++fs | ||
| $(SRC_RASCSI_CORE): $(SRC_PROTOBUF) |
There was a problem hiding this comment.
Why is the SRC_RASCSI_CORE dependent upon SRC_PROTOBUF? I'm not sure I understand this dependency.
There was a problem hiding this comment.
Should this be:
$(OBJ_PROTOBUF): $(SRC_PROTOBUF)
Then remove $(SRC_PROTOBUF) from RASCSI, RASCTL and RASCSI_TEST as a dependency?
There was a problem hiding this comment.
Without this dependency you get compile time errors because rascsi_interface.pb.h is not yet present when needed to compile the rascsi sources (SRC_RASCSI_CORE) with a parallel make on a multi-core machine:
protobuf_serializer.cpp:10:10: fatal error: rascsi_interface.pb.h: No such file or directory
10 | #include "rascsi_interface.pb.h"
| ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [Makefile:156: obj/fullspec/protobuf_serializer.o] Error 1
|
Thanks @uweseimet ! Approved! |
|
@akuker Thank you for reviewing! Regarding the next release, feature_merge_file_support contains the last set of changes I would like to be part of it. (PR before the weekend.) No functional changes I'm afraid, but hey, if you want less issues, good test coverage and a lot of unit tests, you cannot get all this without testable units of code, or by just adding new functionality. I know that it is time-consuming to review these changes, but if you compare it with the time I have spent on getting the code in better shape and adding those 136 unit tests, which cover a great portion of the non-legacy code ... Some of the open tickets, which are dealing with new backend functionality, will profit from all this. |
commit a304382 Author: Uwe Seimet <48174652+uweseimet@users.noreply.github.com> Date: Thu Oct 6 16:15:19 2022 +0200 Moved rascsi/rasctl specific classes to sub-folders, cleaned up code, fixed SonarCloud issues (#889) * Moved rasctl/rascsi core code to folders * Improved granularity in order to add more unit tests * Pointer handling update * Updated ID and controller handling * Updated memory management * Added unit tests * Fixed SonarCloud issues commit 52259c3 Author: Daniel Markstedt <markstedt@gmail.com> Date: Wed Oct 5 14:14:48 2022 -0700 Improve the logic for checking the network bridge configuration (#894) * Improve the logic for checking the network bridge configuration
… fixed SonarCloud issues (#889) * Moved rasctl/rascsi core code to folders * Improved granularity in order to add more unit tests * Pointer handling update * Updated ID and controller handling * Updated memory management * Added unit tests * Fixed SonarCloud issues
As part of this change I moved some code from command_util.cpp to command_context.cpp, because this cleans up dependencies. Now rasctl.cpp does not any longer depend on the localizer and the rasctl binary has half the size than before. Before there was an indirect dependency.
@akuker Let's make it the standard procedure that I merge my PRs myself after approval. Usually my PRs consist of numerous small commits, and for the merge message I'd like to clean the generated summary up in order to make the final message more concise and less noisy.