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

fix: Stop using displaydoc #8614

Merged
merged 2 commits into from
Jun 17, 2024
Merged

fix: Stop using displaydoc #8614

merged 2 commits into from
Jun 17, 2024

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
83 checks passed
@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.

3 participants