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 code analysis errors #1286

Merged
merged 2 commits into from
Dec 8, 2023
Merged

Fix code analysis errors #1286

merged 2 commits into from
Dec 8, 2023

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Dec 8, 2023

  • Add static when possible
  • Remove unused private members
  • Remove unused out parameters

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Dec 8, 2023
@JamesNK
Copy link
Member Author

JamesNK commented Dec 8, 2023

I don't know who owns all these areas so I added GH suggested reviewers 😋

drewnoakes
drewnoakes approved these changes Dec 8, 2023
View reviewed changes
Copy link
Member

@drewnoakes drewnoakes left a comment

Choose a reason for hiding this comment

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

Thanks for filing this. I was just about to.

src/Aspire.Hosting/Dcp/ApplicationExecutor.cs Show resolved Hide resolved
@JamesNK JamesNK mentioned this pull request Dec 8, 2023
Merged
@JamesNK JamesNK merged commit 64d1dd1 into main Dec 8, 2023
8 checks passed
@JamesNK JamesNK deleted the jamesnk/analysis-errors branch December 8, 2023 06:11
@eerhardt
Copy link
Member

eerhardt commented Dec 8, 2023

Is there a reason these code analysis errors weren't being caught in CI? Anyway we can enable them to be checked so they don't get reintroduced elsewhere?

@drewnoakes
Copy link
Member

My understanding is they're IDE* diagnostics that only show in VS. I don't know why that's the case. They only appeared for me after updating VS.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@drewnoakes drewnoakes drewnoakes approved these changes

@mitchdenny mitchdenny Awaiting requested review from mitchdenny mitchdenny is a code owner

@bwateratmsft bwateratmsft Awaiting requested review from bwateratmsft

Assignees
No one assigned
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JamesNK @eerhardt @drewnoakes