Skip to content

Fix Spotbugs in azure-core #16500

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
Merged
g2vinay merged 3 commits into from
Oct 24, 2020
Merged

Fix Spotbugs in azure-core #16500

g2vinay merged 3 commits into from
Oct 24, 2020

Conversation

@g2vinay
Copy link
Member

@g2vinay g2vinay commented Oct 19, 2020

No description provided.

@ghost ghost added the Azure.Core azure-core label Oct 19, 2020
@g2vinay g2vinay marked this pull request as ready for review October 23, 2020 17:35
@g2vinay g2vinay requested a review from conniey October 23, 2020 17:37
conniey
conniey approved these changes Oct 23, 2020
View reviewed changes
@g2vinay g2vinay closed this Oct 24, 2020
@g2vinay g2vinay reopened this Oct 24, 2020
@g2vinay g2vinay merged commit 12b5df0 into Azure:master Oct 24, 2020
@JonathanGiles
Copy link
Member

JonathanGiles commented Oct 26, 2020

This PR is lacking clarity on why this change was made. Without context, all I am seeing is that our logging will be slower now, and I don't know why. Future developers will not have any context, either in the code or in this PR. Please be sure to create PRs with proper descriptions of the intent of the change, and ideally include appropriate inline code comments about why things are being done the way they are.

All reviewers too - this should be baseline expectations to ensure we are not doing our future selves and teammates a disservice by under-documenting things.

@g2vinay
Copy link
Member Author

g2vinay commented Oct 26, 2020

This PR is lacking clarity on why this change was made. Without context, all I am seeing is that our logging will be slower now, and I don't know why. Future developers will not have any context, either in the code or in this PR. Please be sure to create PRs with proper descriptions of the intent of the change, and ideally include appropriate inline code comments about why things are being done the way they are.

All reviewers too - this should be baseline expectations to ensure we are not doing our future selves and teammates a disservice by under-documenting things.

The descriptions are limited intentionally.

@g2vinay g2vinay deleted the fix-spotbugs-azure-core branch July 13, 2023 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anuchandy anuchandy anuchandy approved these changes

@conniey conniey conniey approved these changes

@alzimmermsft alzimmermsft alzimmermsft approved these changes

@srnagar srnagar srnagar approved these changes

@hemanttanwar hemanttanwar Awaiting requested review from hemanttanwar

@jianghaolu jianghaolu Awaiting requested review from jianghaolu

@JonathanGiles JonathanGiles Awaiting requested review from JonathanGiles

@mssfang mssfang Awaiting requested review from mssfang

Assignees

No one assigned

Labels

Azure.Core azure-core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@g2vinay @JonathanGiles @anuchandy @conniey @alzimmermsft @srnagar