-
Notifications
You must be signed in to change notification settings - Fork 4
Update mtlsUtils.cpp #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Update mtlsUtils.cpp #170
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates mtlsUtils.cpp by adding a new first line before the existing license header.
Changes:
- Inserted a leading
//line above the file’s license header
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,3 +1,4 @@ | |||
| // | |||
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stray // line before the license header makes this file inconsistent with the rest of the component (other files start the /*############################################################################## header at line 1) and may break automated license/header scanners that expect the header at the top of file. Please remove this extra line so the license block is the first content in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,4 +1,4 @@ | |||
| #!/bin/sh | |||
| #!/bin/sh | |||
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shebang must start at the very first character of the file. This line currently has a leading space ( #!/bin/sh), which can prevent the script from running when executed directly (e.g., ./cov_build.sh) and may confuse tooling. Remove the leading whitespace so the file begins with #!/bin/sh at column 1.
| #!/bin/sh | |
| #!/bin/sh |
| @@ -1,3 +1,4 @@ | |||
| // | |||
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds an empty // comment line before the license header. It doesn't convey information and adds noise to the file header; please remove it unless there's a specific tooling requirement it addresses (in which case add a brief explanatory comment instead).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,3 +1,4 @@ | |||
| // | |||
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A standalone // line was added before the file’s existing license/header block. This is inconsistent with other source files in this component (which start directly with the license block) and can interfere with automated license/header detection tools; remove the extra line or incorporate it into the existing header comment if needed.
cov_build.sh
Outdated
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop | ||
| cp /usr/rdk_logger/include/* /usr/local/include |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git clone will fail on re-runs if /usr/rdk_logger already exists (unlike the common_utilities section which removes the directory first). Add a cleanup step (e.g., rm -rf rdk_logger) or switch to git -C ... fetch/reset to make the script idempotent.
| cd $RFC_ROOT | ||
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This step only copies headers from rdk_logger into /usr/local/include, but the build links rfcMgr against -lrdkloggers (see rfcMgr/Makefile.am). Without building/installing the rdk_logger library into the link path, the build can still fail at link time; consider adding a build/install step for rdk_logger (or installing the appropriate package) rather than only copying headers.
| cd $RFC_ROOT | |
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop | |
| cd $RFC_ROOT | |
| rm -rf rdk_logger | |
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop | |
| cd rdk_logger | |
| autoreconf -i | |
| ./configure --prefix=${RFC_INSTALL_DIR} | |
| make && make install | |
| cd $RFC_ROOT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,3 +1,4 @@ | |||
|
|
|||
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An empty line has been added at the start of the file, before the shebang. This is incorrect - the shebang (#!/bin/sh) must be the first line of the file with no preceding whitespace or blank lines. Otherwise, the shell script may not be interpreted correctly by the system.
|
|
||
| cd $RFC_ROOT | ||
|
|
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two consecutive blank lines added here (lines 37 and 39) followed by a redundant directory change command. The extra blank lines serve no purpose and reduce code readability. Additionally, the 'cd $RFC_ROOT' command on line 38 is redundant since the script is already in that directory from line 33, and there was no directory change in between.
| cd $RFC_ROOT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop | ||
| cp rdk_logger/include/rdk_logger.h /usr/local/include | ||
|
|
||
|
|
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are extra blank lines (lines 37-38) that add unnecessary whitespace. Consider removing these to maintain consistency with the rest of the codebase.
| fi | ||
|
|
||
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop | ||
| cp rdk_logger/include/* /usr/local/include |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script uses a wildcard to copy all files from rdk_logger/include/* while cov_build.sh (line 35) only copies rdk_logger/include/rdk_logger.h. This inconsistency could lead to different behavior between unit test and coverage build environments. Consider standardizing the approach - either both should copy all headers or both should copy only rdk_logger.h.
| cp rdk_logger/include/* /usr/local/include | |
| cp rdk_logger/include/rdk_logger.h /usr/local/include |
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop | ||
| cp rdk_logger/include/rdk_logger.h /usr/local/include | ||
|
|
||
|
|
||
| cd $RFC_ROOT | ||
|
|
||
| rm -rf common_utilities | ||
| git clone https://github.com/rdkcentral/common_utilities.git -b develop |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The git clone command lacks error handling. If the clone fails (e.g., due to network issues or if the directory already exists from a previous run), the script will continue execution and likely fail later with unclear errors. Consider adding error checking after the git clone command, such as checking the exit code or using set -e at the script level.
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop | |
| cp rdk_logger/include/rdk_logger.h /usr/local/include | |
| cd $RFC_ROOT | |
| rm -rf common_utilities | |
| git clone https://github.com/rdkcentral/common_utilities.git -b develop | |
| if ! git clone https://github.com/rdkcentral/rdk_logger.git -b develop; then | |
| echo "Error: Failed to clone rdk_logger repository" >&2 | |
| exit 1 | |
| fi | |
| cp rdk_logger/include/rdk_logger.h /usr/local/include | |
| cd $RFC_ROOT | |
| rm -rf common_utilities | |
| if ! git clone https://github.com/rdkcentral/common_utilities.git -b develop; then | |
| echo "Error: Failed to clone common_utilities repository" >&2 | |
| exit 1 | |
| fi |
| @@ -1,3 +1,4 @@ | |||
| // | |||
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An empty comment line has been added. This appears to serve no purpose and should be removed for code cleanliness.
rfcMgr/gtest/mocks/rdk_debug.h
Outdated
|
|
||
|
|
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra blank lines (34-35) have been added before the conditional compilation block. Consider removing these to maintain consistency with the rest of the file.
| ./configure --prefix=${RFC_INSTALL_DIR} --enable-rfctool=yes --enable-tr181set=yes | ||
|
|
||
| cd $RFC_ROOT | ||
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The git clone command will fail if the rdk_logger directory already exists from a previous run. Consider adding a cleanup step (e.g., rm -rf rdk_logger) before cloning, or using a conditional check to skip cloning if the directory already exists. This is especially important for CI/CD environments where the script may be run multiple times.
| ENABLE_COV=true | ||
| fi | ||
|
|
||
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The git clone command will fail if the rdk_logger directory already exists from a previous run. Consider adding a cleanup step (e.g., rm -rf rdk_logger) before cloning, or using a conditional check to skip cloning if the directory already exists. This is especially important for CI/CD environments where the script may be run multiple times.
| @@ -1,3 +1,4 @@ | |||
|
|
|||
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty line added before the shebang. The shebang should be on the first line of the file for proper script execution. Remove this empty line.
| ENABLE_COV=true | ||
| fi | ||
|
|
||
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The git clone command lacks error handling. If the clone fails (e.g., due to network issues or if the directory already exists), the script will continue execution and likely fail later with unclear errors. Consider adding error checking after the git clone command, such as checking the exit code or using set -e at the script level.
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop | |
| git clone https://github.com/rdkcentral/rdk_logger.git -b develop | |
| if [ $? -ne 0 ]; then | |
| echo "Error: Failed to clone rdk_logger repository" | |
| exit 1 | |
| fi |
No description provided.