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

style: use std::fmt instead of importing Debug and Display #1087

Merged
merged 1 commit into from
May 5, 2024
Merged

Conversation

joshka
Copy link
Member

@joshka joshka commented May 5, 2024

Based on a discussion with @EdJoPaTo on a PR.

This is a small universal style change to avoid making this change a part of other PRs.

Copy link

codecov bot commented May 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.1%. Comparing base (5f1e119) to head (de8fd50).

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1087   +/-   ##
=====================================
  Coverage   94.1%   94.1%           
=====================================
  Files         61      61           
  Lines      14619   14619           
=====================================
  Hits       13764   13764           
  Misses       855     855           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joshka
Copy link
Member Author

joshka commented May 5, 2024

Lint error is fixed in #1088

@joshka joshka merged commit aa4260f into main May 5, 2024
33 checks passed
@joshka joshka deleted the jm/fmt branch May 5, 2024 06:51
@EdJoPaTo
Copy link
Member

EdJoPaTo commented May 5, 2024

I would have went with std::fmt::Debug as it only affects two likes to use std::fmt instead of fmt and is a bit more concise on what exactly was impl here.

also side note: merging quickly prevents more feedback. Especially when PR are open for weeks normally.

@joshka
Copy link
Member Author

joshka commented May 5, 2024

I would have went with std::fmt::Debug as it only affects two likes to use std::fmt instead of fmt and is a bit more concise on what exactly was impl here.

use std::fmt comes directly from the docs, which I'd treat as idiomatic (as well as being aligned with what I see as subjectively tasteful).

also side note: merging quickly prevents more feedback. Especially when PR are open for weeks normally.

Agreed. Apologies. I should have waited. Ideally PRs shouldn't be hitting weeks, but I understand that's been happening a bit more than usual lately. To some extent all the other maintainers have had various travel plans / other stuff going on in their life.

joshka added a commit to nowNick/ratatui that referenced this pull request May 24, 2024
…rg#1087)

This is a small universal style change to avoid making this change a
part of other PRs.

[rationale](ratatui-org#1083 (comment))
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.

None yet

3 participants