PengZheng commented on code in PR #840:
URL: https://github.com/apache/celix/pull/840#discussion_r3079487341
##########
examples/celix-examples/dm_example/phase2a/src/phase2a_activator.c:
##########
@@ -56,7 +56,7 @@ static celix_status_t activator_start(struct
phase2a_activator_struct *act, celi
celix_dmServiceDependency_setService(dep, PHASE1_NAME,
PHASE1_RANGE_ALL, NULL);
celix_dmServiceDependency_setCallback(dep, (void*)phase2a_setPhase1);
celix_dmServiceDependency_setStrategy(dep,
DM_SERVICE_DEPENDENCY_STRATEGY_LOCKING);
- celix_dmServiceDependency_setRequired(dep, true);
Review Comment:
I just noticed that this removed interface is still mentioned in our
documentation. Please update them.
##########
libs/framework/benchmark/src/DependencyManagerBenchmark.cc:
##########
@@ -17,8 +17,10 @@
* under the License.
*/
-#include <benchmark/benchmark.h>
+#include "../../include/celix_dm_service_dependency.h"
Review Comment:
We should rely on CMake target to setup include path. Relative path is
fragile at best.
##########
libs/framework/include/celix_dm_info.h:
##########
@@ -43,6 +43,7 @@ struct celix_dm_service_dependency_info_struct {
char *filter;
char *versionRange;
bool available;
+ size_t minimalCardinality;
Review Comment:
Considering memory layout, this arrangement is suboptimal, e.g., the
following is better:
```C
struct celix_dm_service_dependency_info_struct {
char *serviceName;
char *filter;
char *versionRange;
size_t count;
size_t minimalCardinality;
bool available;
bool required;
};
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
