Skip to content
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

Fix coverity hits in Classifier, Decision Forest and GBT #3001

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Vika-F
Copy link
Contributor

@Vika-F Vika-F commented Dec 2, 2024

Rule of three violations were fixed mostly by adding default implementations or deleting copy constructors and assignment operators.

The default implementations were added into Parameter, Input, InputIface, Model, ModelImpl classes as they are meant to be copy-assignable and copy-constructible.

Non-default implementations of copy constructor and assignment operator were added in GBT ModelImpl and multiclass classifier Model classes because they contain non-POD members.

In the internal classes that do not need to be copyable the copy constructors and assignment operators were deleted.

PR completeness and readability

  • I have reviewed my changes thoroughly before submitting this pull request.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have added a respective label(s) to PR if I have a permission for that.
  • I have resolved any merge conflicts that might occur with the base branch.

Testing

  • I have run it locally and tested the changes extensively.
  • All CI jobs are green or I have provided justification why they aren't.

@Vika-F Vika-F changed the title Fix coverity hist in GBT Fix coverity hits in GBT Dec 2, 2024
@Vika-F Vika-F changed the title Fix coverity hits in GBT Fix coverity hits in Classifier, Decision Forest and GBT Dec 3, 2024
@Vika-F
Copy link
Contributor Author

Vika-F commented Dec 4, 2024

/intelci: run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant