Skip to content

Conversation

@jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Sep 5, 2025

Issue Addressed

The default debug output of these types contains a lot of unnecessary noise making it hard to read.

This PR removes the type and extra fields from debug output to make logs easier to read.

len could be potentially useful in some cases, but this gives us flexibility to only log it separately if we need it.

Related PR in ssz_types:

RuntimeVariableList

Before

Before:

RuntimeVariableList { vec: [42, 42, 42, 42], max_len: 4 }

After:

[42, 42, 42, 42]

RuntimeFixedVector

Before

RuntimeFixedVector { vec: [42, 42, 42, 42], len: 4 }

After

[42, 42, 42, 42]

@jimmygchen jimmygchen marked this pull request as ready for review September 5, 2025 04:34
@jimmygchen jimmygchen added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! labels Sep 5, 2025
@mergify
Copy link

mergify bot commented Sep 5, 2025

Some required checks have failed. Could you please take a look @jimmygchen? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. ready-for-review The code is ready for review and removed ready-for-review The code is ready for review waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Sep 5, 2025
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

The max_len doesn't seem as nasty as the typenum we were logging in ssz_types, but happy to remove it anyway.

I guess having it could sometimes be useful for debugging config issues? Similar to what Pawan suggested on the ssz_types PR:

@jimmygchen
Copy link
Member Author

The max_len doesn't seem as nasty as the typenum we were logging in ssz_types, but happy to remove it anyway.

I guess having it could sometimes be useful for debugging config issues? Similar to what Pawan suggested on the ssz_types PR:

Yep I can implement something like

[42, 42, 42, 42] (max_len=4)
[42, 42, 42, 42] (len=4)

@jimmygchen jimmygchen added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 9, 2025
@mergify mergify bot added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Sep 9, 2025
@mergify
Copy link

mergify bot commented Sep 9, 2025

Some required checks have failed. Could you please take a look @jimmygchen? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 9, 2025
@mergify mergify bot added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Sep 9, 2025
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Looks nice

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Sep 10, 2025
mergify bot added a commit that referenced this pull request Sep 10, 2025
@mergify
Copy link

mergify bot commented Sep 10, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You can check the last failing draft PR here: #8019.

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@michaelsproul
Copy link
Member

@mergify requeue

@mergify
Copy link

mergify bot commented Sep 10, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

mergify bot added a commit that referenced this pull request Sep 10, 2025
@mergify mergify bot merged commit 811eccd into sigp:unstable Sep 10, 2025
37 checks passed
PoulavBhowmick03 pushed a commit to PoulavBhowmick03/lighthouse that referenced this pull request Sep 12, 2025
The default debug output of these types contains a lot of unnecessary noise making it hard to read.

This PR removes the type and extra fields from debug output to make logs easier to read.

`len` could be potentially useful in some cases, but this gives us flexibility to only log it separately if we need it.

Related PR in `ssz_types`:
- sigp/ssz_types#57


  


Co-Authored-By: Jimmy Chen <[email protected]>
kevaundray pushed a commit to kevaundray/lighthouse that referenced this pull request Sep 13, 2025
The default debug output of these types contains a lot of unnecessary noise making it hard to read.

This PR removes the type and extra fields from debug output to make logs easier to read.

`len` could be potentially useful in some cases, but this gives us flexibility to only log it separately if we need it.

Related PR in `ssz_types`:
- sigp/ssz_types#57


  


Co-Authored-By: Jimmy Chen <[email protected]>
jtraglia pushed a commit to jtraglia/lighthouse that referenced this pull request Sep 16, 2025
The default debug output of these types contains a lot of unnecessary noise making it hard to read.

This PR removes the type and extra fields from debug output to make logs easier to read.

`len` could be potentially useful in some cases, but this gives us flexibility to only log it separately if we need it.

Related PR in `ssz_types`:
- sigp/ssz_types#57


  


Co-Authored-By: Jimmy Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

low-hanging-fruit Easy to resolve, get it before someone else does! ready-for-merge This PR is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants