You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2021/01/04 14:08:56 UTC

[GitHub] [celix] dhbfischer opened a new pull request #311: Added member initialization for framework test to fix building with -Weffc++ and/or -Werror

dhbfischer opened a new pull request #311:
URL: https://github.com/apache/celix/pull/311


   Added constructors for DependencyManagerTestSuite to prevent not initializing the handle attribute and failure during build with -Weffc++ and/or -Werror.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [celix] Oipo merged pull request #311: Added member initialization for framework test to fix building with -Weffc++ and/or -Werror

Posted by GitBox <gi...@apache.org>.
Oipo merged pull request #311:
URL: https://github.com/apache/celix/pull/311


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [celix] codecov-io commented on pull request #311: Added member initialization for framework test to fix building with -Weffc++ and/or -Werror

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #311:
URL: https://github.com/apache/celix/pull/311#issuecomment-754074465


   # [Codecov](https://codecov.io/gh/apache/celix/pull/311?src=pr&el=h1) Report
   > Merging [#311](https://codecov.io/gh/apache/celix/pull/311?src=pr&el=desc) (4be5d2c) into [master](https://codecov.io/gh/apache/celix/commit/0870cd960acae2f9ba74e444e22c7d4f32a1de0a?el=desc) (0870cd9) will **increase** coverage by `0.00%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/311/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/311?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #311   +/-   ##
   =======================================
     Coverage   68.69%   68.69%           
   =======================================
     Files         149      149           
     Lines       30639    30639           
   =======================================
   + Hits        21046    21048    +2     
   + Misses       9593     9591    -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/311?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...dmin\_websocket/src/pubsub\_websocket\_topic\_sender.c](https://codecov.io/gh/apache/celix/pull/311/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX2FkbWluX3dlYnNvY2tldC9zcmMvcHVic3ViX3dlYnNvY2tldF90b3BpY19zZW5kZXIuYw==) | `83.24% <0.00%> (+1.11%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/311?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/311?src=pr&el=footer). Last update [0870cd9...4be5d2c](https://codecov.io/gh/apache/celix/pull/311?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [celix] dhbfischer closed pull request #311: Added member initialization for framework test to fix building with -Weffc++ and/or -Werror

Posted by GitBox <gi...@apache.org>.
dhbfischer closed pull request #311:
URL: https://github.com/apache/celix/pull/311


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [celix] dhbfischer commented on a change in pull request #311: Added member initialization for framework test to fix building with -Weffc++ and/or -Werror

Posted by GitBox <gi...@apache.org>.
dhbfischer commented on a change in pull request #311:
URL: https://github.com/apache/celix/pull/311#discussion_r551357062



##########
File path: libs/framework/gtest/src/DependencyManagerTestSuite.cc
##########
@@ -114,6 +114,10 @@ TEST_F(DependencyManagerTestSuite, StartDmWillBuildCmp) {
 }
 
 struct TestService {
+public:
+    TestService() : handle(nullptr) {};
+    TestService(void *_handle) : handle(_handle) {};

Review comment:
       Thanks, I've edited the implementation.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [celix] Oipo commented on a change in pull request #311: Added member initialization for framework test to fix building with -Weffc++ and/or -Werror

Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #311:
URL: https://github.com/apache/celix/pull/311#discussion_r551343583



##########
File path: libs/framework/gtest/src/DependencyManagerTestSuite.cc
##########
@@ -114,6 +114,10 @@ TEST_F(DependencyManagerTestSuite, StartDmWillBuildCmp) {
 }
 
 struct TestService {
+public:
+    TestService() : handle(nullptr) {};
+    TestService(void *_handle) : handle(_handle) {};

Review comment:
       It's a bit of a nitpick, but these constructors are not necessary. It's a [best practice](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-zero) to not define constructors/destructors if possible. Modify line 121 to `void *handle{nullptr};` instead.
   
   Consequently, modify all initializations of TestService to `TestService svc{};` (e.g. line 162).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org