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

Added rascsi filter to limit logging to a particular device #978

Merged
merged 69 commits into from
Nov 11, 2022

Conversation

uweseimet
Copy link
Contributor

@uweseimet uweseimet commented Nov 10, 2022

Also added more unit tests, resolved SonarQube issues, improved code sharing, improved the error handling of the parser and fixed #979.

@uweseimet uweseimet linked an issue Nov 10, 2022 that may be closed by this pull request
@uweseimet uweseimet changed the title Added a filter to restrict logging (as far as possible) to a particular device Added rascsi filter to restrict logging (as far as possible) to a particular device Nov 10, 2022
@uweseimet uweseimet changed the title Added rascsi filter to restrict logging (as far as possible) to a particular device Added rascsi filter to limit logging (as far as possible) to a particular device Nov 10, 2022
@uweseimet uweseimet marked this pull request as ready for review November 10, 2022 11:33
@@ -146,3 +145,38 @@ bool Device::Eject(bool force)

return true;
}

void Device::LogTrace(const string& error) const
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we come up with a different name? I think it could be confusing to have "LogTrace" and "LOGTRACE". Its not immediately obvious what the difference is.

Copy link
Contributor Author

@uweseimet uweseimet Nov 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with what you say, but I did not know better names. But anyway, by introducing the new DeviceLogger class this issue is resolved, and using a logger object is a better approach in other respects as well. It is now also used by the controller.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thx!

cpp/devices/device.cpp Outdated Show resolved Hide resolved
@uweseimet uweseimet linked an issue Nov 11, 2022 that may be closed by this pull request
@uweseimet uweseimet marked this pull request as ready for review November 11, 2022 16:37
@uweseimet
Copy link
Contributor Author

Ready for review again

@sonarcloud
Copy link

sonarcloud bot commented Nov 11, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

60.2% 60.2% Coverage
0.0% 0.0% Duplication

@akuker
Copy link
Member

akuker commented Nov 11, 2022

@uweseimet - Ready to merge!

@uweseimet uweseimet merged commit 454c61a into develop Nov 11, 2022
@uweseimet uweseimet deleted the feature-log-filter branch November 11, 2022 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SCCD has lost the "removable" property Configurable log level for a particular device ID and LUN
2 participants