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

chore: improve logging in StackerDBListener #5747

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

obycode
Copy link
Contributor

@obycode obycode commented Jan 27, 2025

Log the approved weight and the rejected weight and make the naming consistent.

Log the approved weight and the rejected weight and make the naming
consistent.
@obycode obycode requested a review from a team as a code owner January 27, 2025 15:10
@obycode
Copy link
Contributor Author

obycode commented Jan 27, 2025

Would it be better to just log the percentages instead of the raw numbers here? I'm realizing for me, having just the percentages would be nicer.

@obycode obycode added this to the 3.1.0.0.5 milestone Jan 27, 2025
@obycode
Copy link
Contributor Author

obycode commented Jan 27, 2025

Ok, I added the percentages as well in aa1adf5.

@obycode obycode force-pushed the chore/signer-coordinator-logging branch from aa1adf5 to c03f633 Compare January 28, 2025 15:02
@obycode obycode requested review from jferrant and hstove January 28, 2025 16:11
jferrant
jferrant previously approved these changes Jan 29, 2025
@jferrant
Copy link
Collaborator

Just a general question, sometimes I see we update changelog with logging improvements. Would this be necessary for this PR? Or is it enough to just say "Various logging improvements" under the changelog?

kantai
kantai previously approved these changes Jan 29, 2025
@obycode
Copy link
Contributor Author

obycode commented Jan 29, 2025

Just a general question, sometimes I see we update changelog with logging improvements. Would this be necessary for this PR? Or is it enough to just say "Various logging improvements" under the changelog?

My general thinking is that logging changes are not changelog worthy unless it's something significant like a major cleanup of logs, or addressing some specific pain point.

@kantai
Copy link
Member

kantai commented Jan 29, 2025

Just a general question, sometimes I see we update changelog with logging improvements. Would this be necessary for this PR? Or is it enough to just say "Various logging improvements" under the changelog?

Yeah, I'd think just adding Various logging improvements under the changelog?

@obycode obycode added this pull request to the merge queue Jan 29, 2025
@obycode
Copy link
Contributor Author

obycode commented Jan 29, 2025

Ok, I'll add that. I'll wait til one of the other PRs with changelog entries merge first to avoid conflicts.

@obycode obycode removed this pull request from the merge queue due to a manual request Jan 29, 2025
@obycode obycode dismissed stale reviews from kantai and jferrant via 3edfbd1 January 29, 2025 20:28
@obycode obycode requested a review from a team as a code owner January 29, 2025 20:28
@obycode obycode requested review from jferrant and kantai January 29, 2025 20:29
@obycode
Copy link
Contributor Author

obycode commented Jan 29, 2025

Changelog entry added in 3edfbd1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants