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

Fix for #45152 #45168

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix for #45152 #45168

wants to merge 2 commits into from

Conversation

samwherever
Copy link
Contributor

@samwherever samwherever commented Mar 7, 2025 •
edited by github-actions bot
Loading

@samwherever samwherever requested review from BillWagner and a team as code owners March 7, 2025 18:10
@dotnetrepoman dotnetrepoman bot added this to the March 2025 milestone Mar 7, 2025
@dotnet-policy-service dotnet-policy-service bot added dotnet-csharp/svc community-contribution Indicates PR is created by someone from the .NET community. labels Mar 7, 2025
adegeo
adegeo requested changes Mar 7, 2025
View reviewed changes
Copy link
Contributor

@adegeo adegeo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for the submission, however, the bug indicates that the diagram is also wrong. So we need that fixed along with the text. I checked the article and the diagram does indeed reference ILight.PowerStatus instead of ILight.Power().

@samwherever
Copy link
Contributor Author

@adegeo woops! sorry about that, i'll get this fixed and file a new PR later.

BillWagner
BillWagner previously approved these changes Mar 10, 2025
View reviewed changes
Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

Thanks again @samwherever

This looks great, and I'll :shipit: now.

@BillWagner BillWagner dismissed their stale review March 10, 2025 14:45

I'd missed Andy's correct comment.

@samwherever
Copy link
Contributor Author

@adegeo, @BillWagner - when i try to open the SVG in illustrator, i get a notification saying "Clipping will be lost on roundtrip to Tiny" and the text isn't appearing. i can recreate the SVG from scratch if you want, but i wanted to ask beforehand.

@adegeo
Copy link
Contributor

adegeo commented Mar 10, 2025

I don't what was used to create it, I've only used InkScape as it's free all around. (To clarify, I didn't make the image)

@samwherever
Copy link
Contributor Author

@adegeo heyo! i tried updating the diagram via illustrator - let me know if it looks alright. sorry for the delay on this!

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

@adegeo adegeo adegeo requested changes

@BillWagner BillWagner BillWagner left review comments

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
community-contribution Indicates PR is created by someone from the .NET community. dotnet-csharp/svc
Projects
None yet
Milestone
March 2025
Development

Successfully merging this pull request may close these issues.

ILight.Power() was typoed as ILight.PowerStatus.
3 participants
@samwherever @adegeo @BillWagner