Skip to content

Fix missing logging for a LUN when the LUN is explicitly specified#1379

Merged
rdmark merged 3 commits intodevelopfrom
issue_1378
Nov 26, 2023
Merged

Fix missing logging for a LUN when the LUN is explicitly specified#1379
rdmark merged 3 commits intodevelopfrom
issue_1378

Conversation

@uweseimet
Copy link
Copy Markdown
Contributor

No description provided.

@uweseimet uweseimet marked this pull request as ready for review November 18, 2023 15:36
@uweseimet uweseimet added the bug Something isn't working label Nov 18, 2023
@sonarqubecloud
Copy link
Copy Markdown

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 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication


void DeviceLogger::Log(level::level_enum level, const string& message) const
{
if (!message.empty() &&
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The !message.empty() check is not needed here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Inded, it is actually counter productive. When you ignore empty log messages you never notice that something with your logging is wrong. Because why would you intentially not log a message? This is very likely a bug, but you will not stumble upon it. If there is (with this change) a timestamp but no message you know that something is wrong with the code logging this message.

@rdmark rdmark merged commit 4d1a10c into develop Nov 26, 2023
@rdmark rdmark deleted the issue_1378 branch November 26, 2023 04:25
rdmark pushed a commit that referenced this pull request May 1, 2024
…1379)

* Fix missing logging for a LUN when the LUN is explicitly specified

* Do not suppress controller messages when LUN is specified

* Fix misleading logging for DaynaPort
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logging for LUN is missing when the LUN is explicitly specified in the log level

2 participants