Skip to content

Conversation

@upbqdn
Copy link
Member

@upbqdn upbqdn commented Jun 17, 2024

Motivation

Rust beta now contains a new lint (rust-lang/rust#120393) which produces a bunch of compilation warnings in the CI. They all come from displaydoc (https://github.com/yaahc/displaydoc) which has an open PR (yaahc/displaydoc#47) containing a trivial fix for the issue but hasn't been merged in more than two months.

Solution

  • Remove displaydoc.
  • Impl fmt::Display manually for types that used to use displaydoc.
  • Unrelated:
    • There's another new lint that checks the alignment in lists in comments, so I fixed that.

Tests

The solution doesn't need tests.

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

upbqdn added 2 commits June 17, 2024 12:24
This commit is unrelated to the solution in this branch. There's a new
lint that produces a warning when lists in comments are not aligned well.
@upbqdn upbqdn added C-bug Category: This is a bug C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG P-High 🔥 rust Pull requests that update Rust code labels Jun 17, 2024
@upbqdn upbqdn self-assigned this Jun 17, 2024
@upbqdn upbqdn requested review from a team as code owners June 17, 2024 10:49
@upbqdn upbqdn requested review from arya2 and removed request for a team June 17, 2024 10:49
@github-actions github-actions bot added the extra-reviews This PR needs at least 2 reviews to merge label Jun 17, 2024
@upbqdn
Copy link
Member Author

upbqdn commented Jun 17, 2024

An alternative solution would be to use the branch of displaydoc containing the fix until it gets merged.

@upbqdn upbqdn removed the extra-reviews This PR needs at least 2 reviews to merge label Jun 17, 2024
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

This looks good, thank you!

An alternative solution would be to use the branch of displaydoc containing the fix until it gets merged.

That may cause issues when we're publishing crates for the next version of Zebra, I like that we're removing a scarcely used dependency, though displaydoc also looks very nice if we want to add it back later and use it more.

@mergify mergify bot merged commit c6f5753 into main Jun 17, 2024
@mergify mergify bot deleted the remove-displaydoc branch June 17, 2024 15:10
@Manishearth
Copy link

Manishearth commented Jun 20, 2024

Displaydoc 0.2.5 was published (yaahc/displaydoc#51), if you wish to go back to mainline displaydoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-bug Category: This is a bug C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG P-High 🔥 rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants