RDKEMW-16743:Integrate to common headers#18
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Removes several unused helper headers from the repository to reduce dead code and avoid maintaining unreferenced utilities.
Changes:
- Deleted unused telemetry helper header.
- Deleted unused file persistence/sync helper header.
- Deleted unused file utility helper header (file move + last-line parsing).
- Deleted unused plugin interface builder helper header.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| helpers/UtilsTelemetry.h | Removed unused telemetry helper wrapper. |
| helpers/UtilssyncPersistFile.h | Removed unused persistence/sync helper functions. |
| helpers/UtilsFile.h | Removed unused file utilities (MoveFile/getLastLine). |
| helpers/PluginInterfaceBuilder.h | Removed unused COM-RPC interface builder/ref helper. |
preeja33
force-pushed
the
feature/RDKEMW-16743
branch
from
90f2b1c to
42de2c9
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
helpers/UtilsFile.h:1
- The PR title/description says 'Remove unused headers', but the diff deletes entire helper utility header files (not just unused
#includes). This is a potentially breaking change because any existing#include \"helpers/UtilsFile.h\"(or the other removed headers) will now hard-fail compilation. Either (mandatory) update the PR title/description to reflect that these headers are being removed from the codebase, or (optional, if backward compatibility is needed) keep thin compatibility headers that forward/include the replacement location or emit a clear#errorwith migration guidance.
preeja33
force-pushed
the
feature/RDKEMW-16743
branch
from
eb3e2c0 to
d6ffffd
Compare
Comment on lines
+105
to
+110
| - name: Checkout entservices-persistentstore | ||
| uses: actions/checkout@v3 | ||
| with: | ||
| repository: rdkcentral/entservices-persistentstore | ||
| path: entservices-persistentstore | ||
| ref: develop | ||
| ref: feature/RDKEMW-16743 |
preeja33
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (7)
.github/workflows/L2-tests.yml:110
- This workflow pins checkouts of entservices-usersettings and entservices-persistentstore to a feature branch (feature/RDKEMW-16743). After merge, CI will keep building that branch (and will fail once it’s removed), instead of the commit under test. Use the default ref for the current repo checkout and pin dependencies to a stable tag/branch or SHA (or make refs workflow inputs).
This issue also appears in the following locations of the same file:
- line 112
- line 334
with:
repository: rdkcentral/entservices-usersettings
path: entservices-usersettings
ref: feature/RDKEMW-16743
- name: Checkout entservices-persistentstore
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-persistentstore
path: entservices-persistentstore
ref: develop
.github/workflows/L2-tests.yml:117
- This workflow pins entservices-testframework to a feature branch (feature/helpers_v1). This is brittle for long-term CI and can cause unexpected changes in test behavior. Please pin to a released tag/commit SHA (or parameterize via workflow inputs) before merging.
- name: Checkout entservices-testframework
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-testframework
path: entservices-testframework
ref: feature/helpers_v1
.github/workflows/L2-tests.yml:340
- entservices-helpers is checked out from a feature branch (feature/RDKEMW-16743). This introduces non-reproducible CI and will break if the branch is removed. Pin to a tag/commit SHA or make the ref a workflow input.
- name: Checkout entservices-helpers
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-helpers
path: entservices-helpers
ref: feature/RDKEMW-16743
.github/workflows/L2-tests-oop.yml:130
- This workflow pins the entservices-usersettings checkout (caller_source=testframework) to feature/RDKEMW-16743. After merge, CI will keep building that branch rather than the commit under test, and will fail if the branch is removed. Prefer checking out the triggering commit for this repo and using stable refs (tags/SHA) for dependencies.
- name: Checkout entservices-usersettings-testframework
if: ${{ inputs.caller_source == 'testframework' }}
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-usersettings
path: entservices-usersettings
ref: feature/RDKEMW-16743
.github/workflows/L2-tests-oop.yml:358
- entservices-helpers is checked out from feature/RDKEMW-16743, making CI non-reproducible and fragile. Pin to a tag/commit SHA or parameterize this ref via workflow inputs.
- name: Checkout entservices-helpers
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-helpers
path: entservices-helpers
ref: feature/RDKEMW-16743
.github/workflows/L1-tests.yml:143
- This workflow pins the entservices-usersettings checkout (caller_source=testframework) to feature/RDKEMW-16743. This is brittle post-merge and may cause CI to test the wrong code. Prefer checking out the triggering commit for this repo, and use stable refs (tags/SHA) for dependencies.
- name: Checkout entservices-usersettings-testframework
if: ${{ inputs.caller_source == 'testframework' }}
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-usersettings
path: entservices-usersettings
ref: feature/RDKEMW-16743
.github/workflows/L1-tests.yml:343
- entservices-helpers is checked out from feature/RDKEMW-16743, making CI dependent on a temporary branch. Pin to a tag/commit SHA or make this ref a workflow input.
- name: Checkout entservices-helpers
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-helpers
path: entservices-helpers
ref: feature/RDKEMW-16743
Comment on lines
100
to
103
| # PLUGIN_USERSETTINGS | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/helpers) | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/../entservices-helpers/helpers) | ||
| set (USERSETTINGS_LIBS ${NAMESPACE}UserSettings ${NAMESPACE}UserSettingsImplementation) | ||
| add_plugin_test_ex(PLUGIN_USERSETTINGS tests/test_UserSettings.cpp "${USERSETTINGS_INC}" "${USERSETTINGS_LIBS}") |
|
|
||
| # PLUGIN_USERSETTINGS | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/helpers) | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/../entservices-helpers/helpers) |
Comment on lines
+128
to
+142
| @@ -139,7 +139,7 @@ jobs: | |||
| with: | |||
| repository: rdkcentral/entservices-usersettings | |||
| path: entservices-usersettings | |||
| ref: develop | |||
| ref: feature/RDKEMW-16743 | |||
Comment on lines
+103
to
+117
| @@ -114,7 +114,7 @@ jobs: | |||
| with: | |||
| repository: rdkcentral/entservices-testframework | |||
| path: entservices-testframework | |||
| ref: 1.0.1 | |||
| ref: feature/helpers_v1 | |||
Comment on lines
+108
to
+129
| @@ -126,7 +126,7 @@ jobs: | |||
| with: | |||
| repository: rdkcentral/entservices-usersettings | |||
| path: entservices-usersettings | |||
| ref: develop | |||
| ref: feature/RDKEMW-16743 | |||
| git clone --branch main https://github.com/rdkcentral/entservices-apis.git | ||
|
|
||
| cd .. | ||
| git clone --branch feature/RDKEMW-16743 https://github.com/rdkcentral/entservices-helpers.git |
Comment on lines
+29
to
+34
| if(WPEFrameworkHelpers_FOUND AND NOT TARGET WPEFramework::WPEFrameworkHelpers) | ||
| add_library(WPEFramework::WPEFrameworkHelpers SHARED IMPORTED) | ||
| set_target_properties(WPEFramework::WPEFrameworkHelpers PROPERTIES | ||
| IMPORTED_LOCATION "${WPEFrameworkHelpers_LIBRARIES}" | ||
| INTERFACE_INCLUDE_DIRECTORIES "${WPEFrameworkHelpers_INCLUDE_DIRS}") | ||
| endif() |
Comment on lines
+10
to
+34
| find_library(WPEFrameworkHelpers_LIBRARIES | ||
| NAMES WPEFrameworkHelpers | ||
| PATH_SUFFIXES wpeframework/plugins) | ||
|
|
||
| find_path(WPEFrameworkHelpers_INCLUDE_DIRS | ||
| NAMES UtilsLogging.h | ||
| PATH_SUFFIXES wpeframework/helpers) | ||
|
|
||
| set(WPEFrameworkHelpers_LIBRARIES | ||
| ${WPEFrameworkHelpers_LIBRARIES} | ||
| CACHE PATH "Path to WPEFrameworkHelpers library") | ||
|
|
||
| set(WPEFrameworkHelpers_INCLUDE_DIRS ${WPEFrameworkHelpers_INCLUDE_DIRS} CACHE PATH "Path to WPEFrameworkHelpers includes") | ||
|
|
||
| include(FindPackageHandleStandardArgs) | ||
| FIND_PACKAGE_HANDLE_STANDARD_ARGS(WPEFrameworkHelpers DEFAULT_MSG | ||
| WPEFrameworkHelpers_INCLUDE_DIRS | ||
| WPEFrameworkHelpers_LIBRARIES) | ||
|
|
||
| if(WPEFrameworkHelpers_FOUND AND NOT TARGET WPEFramework::WPEFrameworkHelpers) | ||
| add_library(WPEFramework::WPEFrameworkHelpers SHARED IMPORTED) | ||
| set_target_properties(WPEFramework::WPEFrameworkHelpers PROPERTIES | ||
| IMPORTED_LOCATION "${WPEFrameworkHelpers_LIBRARIES}" | ||
| INTERFACE_INCLUDE_DIRECTORIES "${WPEFrameworkHelpers_INCLUDE_DIRS}") | ||
| endif() |
Comment on lines
+128
to
+142
| @@ -139,7 +139,7 @@ jobs: | |||
| with: | |||
| repository: rdkcentral/entservices-usersettings | |||
| path: entservices-usersettings | |||
| ref: develop | |||
| ref: feature/RDKEMW-16743 | |||
Comment on lines
+103
to
+117
| @@ -114,7 +114,7 @@ jobs: | |||
| with: | |||
| repository: rdkcentral/entservices-testframework | |||
| path: entservices-testframework | |||
| ref: 1.0.1 | |||
| ref: feature/helpers_v1 | |||
Comment on lines
+108
to
+129
| @@ -126,7 +126,7 @@ jobs: | |||
| with: | |||
| repository: rdkcentral/entservices-usersettings | |||
| path: entservices-usersettings | |||
| ref: develop | |||
| ref: feature/RDKEMW-16743 | |||
| git clone --branch main https://github.com/rdkcentral/entservices-apis.git | ||
|
|
||
| cd .. | ||
| git clone --branch feature/RDKEMW-16743 https://github.com/rdkcentral/entservices-helpers.git |
|
|
||
| # PLUGIN_USERSETTINGS | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/helpers) | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/../entservices-helpers/helpers) |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
.github/workflows/L2-tests-oop.yml:130
- The workflow pins
entservices-usersettingscheckout tofeature/RDKEMW-16743for thetestframeworkcaller path. This means CI may not build/test the code corresponding to the workflow run, and will drift as the feature branch moves. Use the branch/commit that triggered the workflow (or revert to the priordevelop).
- name: Checkout entservices-usersettings-testframework
if: ${{ inputs.caller_source == 'testframework' }}
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-usersettings
path: entservices-usersettings
ref: develop
Comment on lines
100
to
103
| # PLUGIN_USERSETTINGS | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/helpers) | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/../entservices-helpers/helpers) | ||
| set (USERSETTINGS_LIBS ${NAMESPACE}UserSettings ${NAMESPACE}UserSettingsImplementation) | ||
| add_plugin_test_ex(PLUGIN_USERSETTINGS tests/test_UserSettings.cpp "${USERSETTINGS_INC}" "${USERSETTINGS_LIBS}") |
Comment on lines
+1
to
+16
| # - Try to find WPEFrameworkHelpers | ||
| # Once done this will define | ||
| # WPEFrameworkHelpers_FOUND - System has WPEFrameworkHelpers | ||
| # WPEFrameworkHelpers_INCLUDE_DIRS - The WPEFrameworkHelpers include directories | ||
| # WPEFrameworkHelpers_LIBRARIES - The libraries needed to use WPEFrameworkHelpers | ||
| # | ||
| # Also creates an imported target: | ||
| # WPEFramework::WPEFrameworkHelpers | ||
|
|
||
| find_library(WPEFrameworkHelpers_LIBRARIES | ||
| NAMES WPEFrameworkHelpers | ||
| PATH_SUFFIXES wpeframework/plugins) | ||
|
|
||
| find_path(WPEFrameworkHelpers_INCLUDE_DIRS | ||
| NAMES UtilsLogging.h | ||
| PATH_SUFFIXES wpeframework/helpers) |
Comment on lines
66
to
70
| endif (RDK_SERVICES_L2_TEST) | ||
|
|
||
| target_link_libraries(${PLUGIN_IMPLEMENTATION} | ||
| PRIVATE | ||
| CompileSettingsDebug::CompileSettingsDebug |
Comment on lines
+10
to
+14
| find_library(WPEFrameworkHelpers_LIBRARIES | ||
| NAMES WPEFrameworkHelpers | ||
| PATH_SUFFIXES wpeframework/plugins) | ||
|
|
||
| find_path(WPEFrameworkHelpers_INCLUDE_DIRS |
| git clone --branch develop https://github.com/rdkcentral/entservices-helpers.git | ||
| cd "$GITHUB_WORKSPACE" | ||
|
|
||
| git clone --branch 1.0.12 https://github.com/rdkcentral/entservices-testframework.git |
Comment on lines
+136
to
+141
| cmake -G Ninja -S ../entservices-helpers -B build/entservices-helpers \ | ||
| -DEXCEPTIONS_ENABLE=ON \ | ||
| -DCMAKE_INSTALL_PREFIX="$GITHUB_WORKSPACE/install/usr" \ | ||
| -DCMAKE_MODULE_PATH="$GITHUB_WORKSPACE/install/tools/cmake" \ | ||
| "-DCMAKE_CXX_FLAGS=-I$GITHUB_WORKSPACE/entservices-testframework/Tests/mocks -I$GITHUB_WORKSPACE/entservices-testframework/Tests/headers -I$GITHUB_WORKSPACE/entservices-testframework/Tests/headers/rdk/iarmbus -include $GITHUB_WORKSPACE/entservices-testframework/Tests/mocks/Iarm.h " | ||
| cmake --build build/entservices-helpers --target install |
| @@ -69,7 +68,8 @@ endif (RDK_SERVICES_L2_TEST) | |||
| target_link_libraries(${PLUGIN_IMPLEMENTATION} | |||
Comment on lines
+10
to
+12
| find_library(WPEFrameworkHelpers_LIBRARIES | ||
| NAMES WPEFrameworkHelpers | ||
| PATH_SUFFIXES wpeframework/plugins) |
Comment on lines
+14
to
+16
| find_path(WPEFrameworkHelpers_INCLUDE_DIRS | ||
| NAMES UtilsLogging.h | ||
| PATH_SUFFIXES wpeframework/helpers) |
| git clone --branch develop https://github.com/rdkcentral/entservices-helpers.git | ||
| cd "$GITHUB_WORKSPACE" | ||
|
|
||
| git clone --branch 1.0.12 https://github.com/rdkcentral/entservices-testframework.git |
Comment on lines
+361
to
+369
| cmake -G Ninja | ||
| -S "$GITHUB_WORKSPACE/entservices-helpers" | ||
| -B build/entservices-helpers | ||
| -DCMAKE_INSTALL_PREFIX="$GITHUB_WORKSPACE/install/usr" | ||
| -include $GITHUB_WORKSPACE/entservices-testframework/Tests/mocks/tr181api.h | ||
| -DCMAKE_MODULE_PATH="$GITHUB_WORKSPACE/install/tools/cmake" | ||
| -DUSE_THUNDER_R4=ON | ||
| -DHIDE_NON_EXTERNAL_SYMBOLS=OFF | ||
| -DPLUGIN_HELPERS=ON |
Comment on lines
+10
to
+12
| find_library(WPEFrameworkHelpers_LIBRARIES | ||
| NAMES WPEFrameworkHelpers | ||
| PATH_SUFFIXES wpeframework/plugins) |
Comment on lines
+14
to
+16
| find_path(WPEFrameworkHelpers_INCLUDE_DIRS | ||
| NAMES UtilsLogging.h | ||
| PATH_SUFFIXES wpeframework/helpers) |
| WPEFrameworkHelpers_LIBRARIES) | ||
|
|
||
| if(WPEFrameworkHelpers_FOUND AND NOT TARGET WPEFramework::WPEFrameworkHelpers) | ||
| add_library(WPEFramework::WPEFrameworkHelpers SHARED IMPORTED) |
|
|
||
| # PLUGIN_USERSETTINGS | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/helpers) | ||
| set (USERSETTINGS_INC ${CMAKE_SOURCE_DIR}/plugin ${CMAKE_SOURCE_DIR}/../entservices-helpers/helpers) |
| with: | ||
| repository: rdkcentral/entservices-helpers | ||
| path: entservices-helpers | ||
| ref: develop |
| with: | ||
| repository: rdkcentral/entservices-helpers | ||
| path: entservices-helpers | ||
| ref: develop |
|
|
||
| git clone --branch 1.0.1 https://github.com/rdkcentral/entservices-testframework.git | ||
| cd .. | ||
| git clone --branch develop https://github.com/rdkcentral/entservices-helpers.git |
RajaLucy
force-pushed
the
feature/RDKEMW-16743
branch
from
5409b6f to
e501ad5
Compare
Comment on lines
68
to
+72
| target_link_libraries(${PLUGIN_IMPLEMENTATION} | ||
| PRIVATE | ||
| CompileSettingsDebug::CompileSettingsDebug | ||
| ${NAMESPACE}Plugins::${NAMESPACE}Plugins) | ||
| ${NAMESPACE}Plugins::${NAMESPACE}Plugins | ||
| ${NAMESPACE}::${NAMESPACE}Helpers) |
Comment on lines
+10
to
+16
| find_library(WPEFrameworkHelpers_LIBRARIES | ||
| NAMES WPEFrameworkHelpers | ||
| PATH_SUFFIXES wpeframework/plugins) | ||
|
|
||
| find_path(WPEFrameworkHelpers_INCLUDE_DIRS | ||
| NAMES UtilsLogging.h | ||
| PATH_SUFFIXES wpeframework/helpers) |
Comment on lines
+54
to
59
| cd .. | ||
| git clone --branch develop https://github.com/rdkcentral/entservices-helpers.git | ||
| cd "$GITHUB_WORKSPACE" | ||
|
|
||
| git clone --branch 1.0.12 https://github.com/rdkcentral/entservices-testframework.git | ||
|
|
Comment on lines
+337
to
+342
| - name: Checkout entservices-helpers | ||
| uses: actions/checkout@v3 | ||
| with: | ||
| repository: rdkcentral/entservices-helpers | ||
| path: entservices-helpers | ||
| ref: develop |
Comment on lines
+335
to
+340
| - name: Checkout entservices-helpers | ||
| uses: actions/checkout@v3 | ||
| with: | ||
| repository: rdkcentral/entservices-helpers | ||
| path: entservices-helpers | ||
| ref: develop |
RajaLucy
force-pushed
the
feature/RDKEMW-16743
branch
from
19b779b to
e501ad5
Compare
Comment on lines
+361
to
+369
| cmake -G Ninja | ||
| -S "$GITHUB_WORKSPACE/entservices-helpers" | ||
| -B build/entservices-helpers | ||
| -DCMAKE_INSTALL_PREFIX="$GITHUB_WORKSPACE/install/usr" | ||
| -include $GITHUB_WORKSPACE/entservices-testframework/Tests/mocks/tr181api.h | ||
| -DCMAKE_MODULE_PATH="$GITHUB_WORKSPACE/install/tools/cmake" | ||
| -DUSE_THUNDER_R4=ON | ||
| -DHIDE_NON_EXTERNAL_SYMBOLS=OFF | ||
| -DPLUGIN_HELPERS=ON |
Comment on lines
+14
to
+16
| find_path(WPEFrameworkHelpers_INCLUDE_DIRS | ||
| NAMES UtilsLogging.h | ||
| PATH_SUFFIXES wpeframework/helpers) |
Comment on lines
+29
to
+34
| if(WPEFrameworkHelpers_FOUND AND NOT TARGET WPEFramework::WPEFrameworkHelpers) | ||
| add_library(WPEFramework::WPEFrameworkHelpers SHARED IMPORTED) | ||
| set_target_properties(WPEFramework::WPEFrameworkHelpers PROPERTIES | ||
| IMPORTED_LOCATION "${WPEFrameworkHelpers_LIBRARIES}" | ||
| INTERFACE_INCLUDE_DIRECTORIES "${WPEFrameworkHelpers_INCLUDE_DIRS}") | ||
| endif() |
Comment on lines
+54
to
+58
| cd .. | ||
| git clone --branch develop https://github.com/rdkcentral/entservices-helpers.git | ||
| cd "$GITHUB_WORKSPACE" | ||
|
|
||
| git clone --branch 1.0.12 https://github.com/rdkcentral/entservices-testframework.git |
srikanth-vv
approved these changes
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for change: Integrate to common headers
Test Procedure: None
Risks: Low
Priority: P1