Skip to content

Conversation

@macladson
Copy link
Member

Issue Addressed

Another good candidate for publishing separately from Lighthouse is sensitive_url as it's a general utility crate and not related to Ethereum. This PR prepares it to be spun out into its own crate.

Proposed Changes

I've made the full field on SensitiveUrl private and instead provided an explicit getter called .expose_full(). It's a bit ugly for the diff but I prefer the explicit nature of the getter.
I've also added some extra tests and doc strings along with feature gating Serialize and Deserialize implementations behind the serde feature.

Additional Info

I also removed AsRef<str> as this was previously used quite awkwardly and instead I've implemented a .redacted() getter which I think is cleaner.

@macladson macladson added code-quality dependencies Pull requests that update a dependency file labels Oct 16, 2025
@macladson macladson added the ready-for-review The code is ready for review label Oct 16, 2025
@mergify
Copy link

mergify bot commented Oct 16, 2025

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

@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 Oct 16, 2025
@macladson macladson 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 Oct 16, 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.

Nice! Keen!

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

mergify bot commented Nov 4, 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: #8359.

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.

@mergify mergify bot added dequeued and removed queued labels Nov 4, 2025
@michaelsproul michaelsproul removed the ready-for-merge This PR is ready to merge. label Nov 4, 2025
@macladson macladson force-pushed the improve-sensitive-url branch from 2b977ff to 20b74c6 Compare November 4, 2025 10:30
@macladson macladson added the ready-for-review The code is ready for review label Nov 4, 2025
@mergify mergify bot removed the dequeued label Nov 4, 2025
@michaelsproul
Copy link
Member

@mergify requeue

@mergify
Copy link

mergify bot commented Nov 5, 2025

requeue

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

@michaelsproul michaelsproul removed the ready-for-review The code is ready for review label Nov 5, 2025
@michaelsproul michaelsproul added the ready-for-merge This PR is ready to merge. label Nov 5, 2025
@mergify mergify bot added the queued label Nov 5, 2025
mergify bot added a commit that referenced this pull request Nov 5, 2025
@mergify mergify bot merged commit 3066f0b into sigp:unstable Nov 5, 2025
37 checks passed
@mergify mergify bot removed the queued label Nov 5, 2025
@macladson macladson deleted the improve-sensitive-url branch November 5, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-quality dependencies Pull requests that update a dependency file ready-for-merge This PR is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants