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

Fixes two logging source gen bugs - when using "in" or "ref" modifier / when dealing with constraints #64593

Merged
merged 3 commits into from
Feb 2, 2022

Conversation

maryamariyan
Copy link
Member

@maryamariyan maryamariyan commented Feb 1, 2022

  • Supports usage of in or ref modifier
  • Improves support for generic constraints

Fixes #58550, #62644

- Supports usage of "in" modifier
- Improves support for generic constraints

Fixes dotnet#58550, dotnet#62644
@ghost
Copy link

ghost commented Feb 1, 2022

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Supports usage of "in" modifier
  • Improves support for generic constraints

Fixes #58550, #62644

Author: maryamariyan
Assignees: -
Labels:

area-Extensions-Logging

Milestone: -

@maryamariyan maryamariyan changed the title Fixes two logging source gen bugs, when using "in" modifier and when dealing with constraints Fixes two logging source gen bugs - when using "in" modifier / when dealing with constraints Feb 1, 2022
@maryamariyan maryamariyan changed the title Fixes two logging source gen bugs - when using "in" modifier / when dealing with constraints Fixes two logging source gen bugs - when using "in" or "ref" modifier / when dealing with constraints Feb 1, 2022
Copy link
Member

@elinor-fung elinor-fung left a comment

Choose a reason for hiding this comment

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

if the log method has ... out we would not support it and just give out the diagnostic

Is there a separate issue already tracking failing nicely for out?

@@ -86,7 +86,7 @@ namespace {lc.Namespace}
// loop until you find top level nested class
while (parent != null)
{
parentClasses.Add($"partial {parent.Keyword} {parent.Name} {parent.Constraints}");
parentClasses.Add($"partial {parent.Keyword} {parent.Name} ");
Copy link
Member

Choose a reason for hiding this comment

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

nit: you can probably remove the trailing space too

Copy link
Member Author

@maryamariyan maryamariyan Feb 2, 2022

Choose a reason for hiding this comment

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

will fix in the next PR


UPDATE: created #64666

@maryamariyan
Copy link
Member Author

maryamariyan commented Feb 2, 2022

Is there a separate issue already tracking failing nicely for out?

No, but will create it


UPDATE: created #64665

@maryamariyan
Copy link
Member Author

Failures in CI seem unrelated and I also seen them in #64573 and #64641

@maryamariyan
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2022

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1786699177

maryamariyan added a commit to maryamariyan/runtime that referenced this pull request Feb 4, 2022
… / when dealing with constraints (dotnet#64593)

* Fixes some logging source gen bugs:

- Supports usage of "in" modifier
- Improves support for generic constraints

Fixes dotnet#58550, dotnet#62644

* Apply PR feedback

* Add another test
@ghost ghost locked as resolved and limited conversation to collaborators Mar 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants