-
Notifications
You must be signed in to change notification settings - Fork 6
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
Mon 34072 move opt to common #1451
Conversation
WalkthroughThe changes consolidate common functionalities and standardize licensing across files. The project structure is updated by adding common include paths, replacing individual header inclusions with a central common header, adjusting namespace declarations, and switching from GNU GPL to Apache License 2.0. These modifications aim to enhance code maintainability, consistency, and legal clarity. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@coderabbitai review |
Actions performedReview triggered.
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (27)
- broker/neb/CMakeLists.txt (1 hunks)
- common/inc/com/centreon/common/opt.hh (2 hunks)
- engine/CMakeLists.txt (2 hunks)
- engine/inc/com/centreon/engine/configuration/anomalydetection.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/contact.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/contactgroup.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/host.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/hostdependency.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/hostescalation.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/hostgroup.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/service.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/servicedependency.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/serviceescalation.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/servicegroup.hh (1 hunks)
- engine/inc/com/centreon/engine/configuration/timeperiod.hh (1 hunks)
- engine/inc/com/centreon/engine/retention/contact.hh (2 hunks)
- engine/inc/com/centreon/engine/retention/host.hh (2 hunks)
- engine/inc/com/centreon/engine/retention/program.hh (2 hunks)
- engine/inc/com/centreon/engine/retention/service.hh (2 hunks)
- engine/modules/CMakeLists.txt (1 hunks)
- engine/modules/external_commands/CMakeLists.txt (1 hunks)
- engine/src/host.cc (2 hunks)
- engine/src/retention/anomalydetection.cc (1 hunks)
- engine/src/retention/contact.cc (1 hunks)
- engine/src/retention/host.cc (2 hunks)
- engine/src/retention/program.cc (1 hunks)
- engine/src/retention/service.cc (2 hunks)
Files not reviewed due to errors (1)
- engine/modules/CMakeLists.txt (no review received)
Files skipped from review due to trivial changes (10)
- broker/neb/CMakeLists.txt
- engine/inc/com/centreon/engine/configuration/host.hh
- engine/inc/com/centreon/engine/configuration/servicegroup.hh
- engine/inc/com/centreon/engine/configuration/timeperiod.hh
- engine/src/host.cc
- engine/src/retention/anomalydetection.cc
- engine/src/retention/contact.cc
- engine/src/retention/host.cc
- engine/src/retention/program.cc
- engine/src/retention/service.cc
Additional comments not posted (21)
engine/modules/external_commands/CMakeLists.txt (1)
26-26
: The addition of${CMAKE_SOURCE_DIR}/clib/inc
to the include directories and${CMAKE_SOURCE_DIR}/build/centreon-clib/
to the link directories are appropriate. These changes should enhance modularity by clearly separating external command dependencies.common/inc/com/centreon/common/opt.hh (1)
20-29
: The update to the namespace and macro protection (CCC_OPT_HH
) is correctly implemented. Additionally, the comment recommending the use ofstd::optional
is a good practice to guide future development away from custom solutions when standard library features are available.engine/inc/com/centreon/engine/configuration/contactgroup.hh (1)
22-22
: The update of the include directive tocom/centreon/common/opt.hh
is correctly implemented, aligning with the namespace adjustments specified in the PR objectives.engine/inc/com/centreon/engine/configuration/hostgroup.hh (1)
22-22
: Ensure that the new header filecom/centreon/common/opt.hh
is correctly placed and accessible, as it's crucial for the namespace usage.Verification successful
The verification script has confirmed that the file
com/centreon/common/opt.hh
does not exist in the specified directory. This could indicate a few possible issues:
- The file might be located in a different directory.
- The file might be missing and needs to be added to the repository.
To gather more information, let's search the repository to see if the file exists elsewhere or if there are any references to it that might provide clues about its intended location.
The verification script has shown that the file
com/centreon/common/opt.hh
does exist in the repository, specifically in thecommon/inc/com/centreon/common/
directory. Additionally, there are multiple references to this file across various parts of the codebase.Given this information, it appears that the initial verification script may have had an issue with the path resolution. However, the file itself is present and being used in multiple places, indicating it is correctly placed and accessible.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accessibility of the new header file. # Test: Check if the file exists in the common directory. test -f ${CMAKE_SOURCE_DIR}/common/inc/com/centreon/common/opt.hh && echo "File exists" || echo "File does not exist"Length of output: 97
Script:
#!/bin/bash # Search for the file `opt.hh` in the entire repository to see if it exists elsewhere. fd -H 'opt.hh' # Search for references to `opt.hh` in the repository to find any clues about its intended location. rg 'opt.hh'Length of output: 1604
engine/inc/com/centreon/engine/retention/contact.hh (1)
23-23
: The inclusion ofcom/centreon/common/opt.hh
and the use of its namespace should be verified for correctness and consistency with other parts of the project.Also applies to: 27-27
Verification successful
The inclusion of the
com/centreon/common/opt.hh
header file is consistent across multiple files in the project, and the namespace usage is correct.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct use of the new namespace across the project. # Test: Search for incorrect namespace usage. rg --type cpp "using com::centreon::engine::opt" || echo "Namespace usage is correct"Length of output: 113
Script:
#!/bin/bash # Description: Verify the inclusion of the new header file across the project. # Test: Search for the inclusion of the new header file. rg --type-add 'hh:*.hh' '#include "com/centreon/common/opt.hh"' || echo "Header inclusion is consistent"Length of output: 1605
engine/inc/com/centreon/engine/configuration/hostescalation.hh (1)
22-22
: The inclusion ofcom/centreon/common/opt.hh
and the use of its namespace should be verified for correctness and consistency with other parts of the project.Also applies to: 26-26
Verification successful
The inclusion of
com/centreon/common/opt.hh
appears to be consistent across multiple files within the project, indicating that the changes align with the project's architecture. Additionally, the namespace usage was confirmed to be correct.
engine/inc/com/centreon/engine/configuration/hostescalation.hh
engine/inc/com/centreon/engine/configuration/contactgroup.hh
engine/inc/com/centreon/engine/configuration/servicegroup.hh
engine/inc/com/centreon/engine/configuration/timeperiod.hh
engine/inc/com/centreon/engine/configuration/serviceescalation.hh
engine/inc/com/centreon/engine/configuration/hostdependency.hh
engine/inc/com/centreon/engine/configuration/service.hh
engine/inc/com/centreon/engine/configuration/hostgroup.hh
engine/inc/com/centreon/engine/configuration/contact.hh
engine/inc/com/centreon/engine/configuration/anomalydetection.hh
engine/inc/com/centreon/engine/configuration/servicedependency.hh
engine/inc/com/centreon/engine/configuration/host.hh
engine/inc/com/centreon/engine/retention/host.hh
engine/inc/com/centreon/engine/retention/program.hh
engine/inc/com/centreon/engine/retention/service.hh
engine/inc/com/centreon/engine/retention/contact.hh
The review comment is verified and correct.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct use of the new namespace across the project. # Test: Search for incorrect namespace usage. rg --type cpp "using com::centreon::engine::opt" || echo "Namespace usage is correct"Length of output: 113
Script:
#!/bin/bash # Description: Verify the inclusion of the new header across the project. # Test: Search for the inclusion of the new header. rg --type cpp '#include "com/centreon/common/opt.hh"' || echo "Header inclusion is correct"Length of output: 1595
engine/inc/com/centreon/engine/configuration/hostdependency.hh (1)
22-22
: The inclusion ofcom/centreon/common/opt.hh
and the use of its namespace should be verified for correctness and consistency with other parts of the project.Also applies to: 26-26
Verification successful
The namespace usage is correct. Now, let's verify if the newly included header file
com/centreon/common/opt.hh
is necessary and correctly used in the codebase.
The newly included header file
com/centreon/common/opt.hh
is indeed being used across multiple files in the project, including the file under review (hostdependency.hh
). The namespacecom::centreon::common::opt
is also utilized correctly in these files.
engine/inc/com/centreon/engine/configuration/contact.hh
engine/inc/com/centreon/engine/configuration/servicegroup.hh
engine/inc/com/centreon/engine/configuration/timeperiod.hh
engine/inc/com/centreon/engine/configuration/serviceescalation.hh
engine/inc/com/centreon/engine/configuration/servicedependency.hh
engine/inc/com/centreon/engine/configuration/service.hh
engine/inc/com/centreon/engine/configuration/hostgroup.hh
engine/inc/com/centreon/engine/configuration/hostescalation.hh
engine/inc/com/centreon/engine/configuration/hostdependency.hh
engine/inc/com/centreon/engine/configuration/host.hh
engine/inc/com/centreon/engine/configuration/anomalydetection.hh
engine/inc/com/centreon/engine/configuration/contactgroup.hh
engine/inc/com/centreon/engine/retention/contact.hh
engine/inc/com/centreon/engine/retention/service.hh
engine/inc/com/centreon/engine/retention/program.hh
engine/inc/com/centreon/engine/retention/host.hh
This confirms that the inclusion of the header and the use of its namespace are consistent and necessary across the project.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct use of the new namespace across the project. # Test: Search for incorrect namespace usage. rg --type cpp "using com::centreon::engine::opt" || echo "Namespace usage is correct"Length of output: 113
Script:
#!/bin/bash # Description: Verify the usage of the newly included header file across the project. # Test: Search for the usage of entities from the header file. rg --type cpp '#include "com/centreon/common/opt.hh"' -A 10Length of output: 15211
engine/inc/com/centreon/engine/configuration/serviceescalation.hh (2)
22-22
: Ensure that the newly included headercom/centreon/common/opt.hh
is used efficiently in the file. Theusing
directive simplifies namespace usage which is good for readability.Also applies to: 26-26
26-26
: Theusing
directive forcom::centreon::common::opt
is appropriate and improves code clarity by reducing namespace verbosity.engine/inc/com/centreon/engine/configuration/servicedependency.hh (1)
22-22
: Consistent with other configuration files, the inclusion ofcom/centreon/common/opt.hh
and the use of theopt
namespace directive aligns with the project's direction to centralize common functionalities.Also applies to: 26-26
engine/inc/com/centreon/engine/retention/program.hh (1)
23-23
: The inclusion ofcom/centreon/common/opt.hh
and the correspondingusing
directive foropt
is consistent with changes in other parts of the project, which is good for maintaining consistency across the codebase.Also applies to: 26-26
engine/inc/com/centreon/engine/configuration/contact.hh (1)
24-24
: The inclusion ofcom/centreon/common/opt.hh
and the use of theopt
namespace directive incontact.hh
enhances consistency and simplifies access to common types across configuration files.Also applies to: 29-29
engine/inc/com/centreon/engine/retention/host.hh (2)
2-16
: Updated licensing to Apache License, Version 2.0. This change aligns with the project's updated licensing policy.
22-22
: Namespace and include directives updated to reflect the project's restructuring.Also applies to: 26-26
engine/inc/com/centreon/engine/configuration/service.hh (2)
Line range hint
2-16
: Updated licensing to Apache License, Version 2.0. Consistent with the project's updated licensing policy.
22-22
: Namespace and include directives updated to reflect the project's restructuring.Also applies to: 27-27
engine/inc/com/centreon/engine/retention/service.hh (2)
2-16
: Updated licensing to Apache License, Version 2.0. Consistent with the project's updated licensing policy.
22-22
: Namespace and include directives updated to reflect the project's restructuring.Also applies to: 26-26
engine/inc/com/centreon/engine/configuration/anomalydetection.hh (2)
Line range hint
2-16
: Updated licensing to Apache License, Version 2.0. Consistent with the project's updated licensing policy.
23-23
: Namespace and include directives updated to reflect the project's restructuring.Also applies to: 28-28
engine/CMakeLists.txt (1)
50-50
: The inclusion of${CMAKE_SOURCE_DIR}/common/inc
in theinclude_directories
ensures that the common headers are accessible across different parts of the project. This change aligns with the project's restructuring goals as outlined in the PR.
REFS: MON-34072