Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Remove irrelevant StructuredMessageTemplateAttribute - #5266

#5266
Merged
snakefoot merged 1 commit into
NLog:devNLog/NLog:devfrom
saltukkos:annotations-resharpersaltukkos/NLog:annotations-resharperCopy head branch name to clipboard
Jun 26, 2023
Merged

Remove irrelevant StructuredMessageTemplateAttribute#5266
snakefoot merged 1 commit into
NLog:devNLog/NLog:devfrom
saltukkos:annotations-resharpersaltukkos/NLog:annotations-resharperCopy head branch name to clipboard

Conversation

@saltukkos

Copy link
Copy Markdown
Contributor

I found that some overloads in ILogger have the [StructuredMessageTemplate] but they don't actually treat the input as a structured message, instead treating it like a regular string. This leads to false-positives in R# and irrelevant syntax highlighting. I believe these attributes shouldn't be here, as they are only applied to the Debug level (search link) (and Fatal for obsolete version of the method). Note that these methods also don't have [MessageTemplateFormatMethod].

Screenshot 2023-06-26 at 11 44 07

@welcome

welcome Bot commented Jun 26, 2023

Copy link
Copy Markdown

Thanks for opening this pull request!
We will try to review this soon! Please note that pull requests with unit tests are earlier accepted 👼

@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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@snakefoot
snakefoot self-requested a review June 26, 2023 17:26

@snakefoot snakefoot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for reporting this, and resolving it with this pull-request.

@snakefoot
snakefoot merged commit 88bfc77 into NLog:dev Jun 26, 2023
@welcome

welcome Bot commented Jun 26, 2023

Copy link
Copy Markdown

Hooray your first pull request is merged! Thanks for the contribution! Looking for more? 👼 Please check the up-for-grabs issues - thanks!
thanks!

@snakefoot snakefoot added this to the 5.2.1 milestone Jun 26, 2023
@snakefoot

Copy link
Copy Markdown
Contributor

NLog v5.2.1 has now been released - https://www.nuget.org/packages/NLog

Thank you again for taking the time to report and fix this issues.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.