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.

Sign up for GitHub

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

Jump to bottom

Bring parity to dotnet extensions logging generator behavior #5370

Merged
merged 24 commits into from
Sep 3, 2024
Merged

Bring parity to dotnet extensions logging generator behavior #5370

merged 24 commits into from
Sep 3, 2024
+1,206 −604

Conversation

dariusclay
Copy link
Contributor

@dariusclay dariusclay commented Aug 17, 2024 •
edited by RussKie
Loading

Fixes #5221
Fixes #5332
Fixes #5336
Resolves #5178

  • Update message template extraction
  • Align behavior regarding @ symbol in message templates
  • Fix issues looking up ILogger in derived classes
  • Adds support for passing ILogger in primary class constructors
Microsoft Reviewers: Open in CodeFlow

Sorry, something went wrong.

@dariusclay
support @ symbols in logging message templates, and align tempalte ex…
Loading
Loading status checks…
c31c017 
…traction behavior
@dariusclay dariusclay requested a review from geeknoid August 17, 2024 11:55
@dariusclay dariusclay self-assigned this Aug 17, 2024
This was linked to issues Aug 17, 2024
Behavior changes between .NET logging generator and .NET extensions #5332
Closed
LOGGEN031 Diagnostic breaks Serilog object serialization #5336
Closed
@dariusclay
fix correctness issue
Loading
Loading status checks…
0ea09fe
@dariusclay
Handle lookup of ILogger member respecting backwards compatibility wi…
Loading
Loading status checks…
5352563 
…th base logging generator
@dariusclay
undo naming change
Loading
Loading status checks…
8f2ae4c
@dariusclay
undo testing code
Loading
Loading status checks…
c21dfd8
@dariusclay
remove unnecessary project property
Loading
Loading status checks…
fc6c9bd
@dariusclay
clean up spans in tests
Loading
Loading status checks…
522cb95
@dariusclay
rename parameter back to original
Loading
Loading status checks…
4b20465
@dariusclay
remove using

Verified

This commit was signed with the committer’s verified signature. The key has expired.
unstubbable Hendrik Liebau
GPG key ID: F431E9657CEB88AB
Expired
Verified
Learn about vigilant mode
Loading
Loading status checks…
ed3758e
@dariusclay
update generated test classes
Loading
Loading status checks…
4fa2ba2 
use Roslyn 4.4 libs again
@dariusclay dariusclay mentioned this pull request Aug 17, 2024
Closed
@dariusclay dariusclay linked an issue Aug 17, 2024 that may be closed by this pull request
Add support for primary constructors in our Logging generator #5178
Closed
geeknoid
geeknoid reviewed Aug 17, 2024
View reviewed changes
RussKie
RussKie reviewed Aug 18, 2024
View reviewed changes
@RussKie RussKie added the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Aug 18, 2024
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Aug 19, 2024
@RussKie RussKie added the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Aug 20, 2024
@dariusclay
Merge branch 'main' into 5332-logging-generator-behavior-parity

Verified

This commit was signed with the committer’s verified signature. The key has expired.
unstubbable Hendrik Liebau
GPG key ID: F431E9657CEB88AB
Expired
Verified
Learn about vigilant mode
8cca66a
@dariusclay
Bump everything to Roslyn 4.8

Verified

This commit was signed with the committer’s verified signature. The key has expired.
unstubbable Hendrik Liebau
GPG key ID: F431E9657CEB88AB
Expired
Verified
Learn about vigilant mode
Loading
Loading status checks…
765fadd
@dariusclay
Suppress RS1035 until violations can be addressed

Verified

This commit was signed with the committer’s verified signature. The key has expired.
unstubbable Hendrik Liebau
GPG key ID: F431E9657CEB88AB
Expired
Verified
Learn about vigilant mode
Loading
Loading status checks…
d4d13a0
RussKie
RussKie reviewed Aug 28, 2024
View reviewed changes
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

Few more minor comments in addition to few outstanding comments (not sure if you addressed those and forgot to push... ¯\_(ツ)_/¯).

@RussKie RussKie added the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Aug 28, 2024
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Aug 28, 2024
@dariusclay
Copy link
Contributor Author

Few more minor comments in addition to few outstanding comments (not sure if you addressed those and forgot to push... ¯_(ツ)_/¯).

The final comments are being addressed while bringing the code coverage back up to meet the threshold.

@dariusclay dariusclay mentioned this pull request Aug 28, 2024
Closed
@dariusclay
Supressing RS1035 in the source instead of for all generator projects

Verified

This commit was signed with the committer’s verified signature. The key has expired.
unstubbable Hendrik Liebau
GPG key ID: F431E9657CEB88AB
Expired
Verified
Learn about vigilant mode
b6568cc
@dariusclay
update list to include LOGGEN037 and LOGGEN038
733d695
@dariusclay
Use constants in place of magic numbers
ab58b2c
@dariusclay
addressing PR comments
Loading
Loading status checks…
7b8e4c6
@dariusclay
remove extraneous using
Loading
Loading status checks…
ad8cb22
@dariusclay
increase code coverage
Loading
Loading status checks…
1d87e1e
@RussKie
Copy link
Member

RussKie commented Aug 29, 2024

I'm going to review either tomorrow or early next week.

@dariusclay
adding more tests
Loading
Loading status checks…
2780154
@dariusclay
Copy link
Contributor Author

I'm going to review either tomorrow or early next week.

Thanks for everything @RussKie

RussKie
RussKie approved these changes Sep 2, 2024
View reviewed changes
src/Generators/Microsoft.Gen.Logging/Parsing/TemplateProcessor.cs
/// </summary>
/// <remarks> The search skips any sequences of {{ or }}.</remarks>
/// <example>{{prefix{{{Argument}}}suffix}}.</example>
/// <returns>The zero-based index position of the first occurrence of the searched brace; -1 if the searched brace was not found; -2 if the wrong brace was found.</returns>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// <returns>The zero-based index position of the first occurrence of the searched brace; -1 if the searched brace was not found; -2 if the wrong brace was found.</returns>
/// <returns>The zero-based index position of the first occurrence of the searched brace; <see cref="NoBracesFound"/> if the searched brace was not found; <see cref="WrongBraceFound"/> if the wrong brace was found.</returns>

@RussKie
Copy link
Member

RussKie commented Sep 2, 2024

@dariusclay feel free to squash-merge when you're ready.

@dariusclay dariusclay merged commit 3131920 into main Sep 3, 2024
6 checks passed
@dariusclay dariusclay deleted the 5332-logging-generator-behavior-parity branch September 3, 2024 08:40
@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@geeknoid geeknoid

@RussKie RussKie

Assignees

@dariusclay dariusclay

Labels
area-telemetry
Projects
None yet
Milestone
No milestone
3 participants
@dariusclay @RussKie @geeknoid