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

chore: lint and doc cleanup #191

Merged
merged 2 commits into from
Jun 2, 2023
Merged

chore: lint and doc cleanup #191

merged 2 commits into from
Jun 2, 2023

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented May 22, 2023

A few more minor cleanups, mostly in documentation

@nyurik nyurik force-pushed the docs branch 2 times, most recently from 1a17e25 to 7ebdd0a Compare May 22, 2023 06:21
@joshka
Copy link
Member

joshka commented May 22, 2023

I notice a bunch of checks are failing on this.

It looks like this change effectively ignores a bunch of places that could be better written. I wonder if it would be worth leaving the lints for those on rather than ignoring them, with a goal to fix the issues rather than ignore them.

@nyurik
Copy link
Contributor Author

nyurik commented May 24, 2023

I notice a bunch of checks are failing on this.

It looks like this change effectively ignores a bunch of places that could be better written. I wonder if it would be worth leaving the lints for those on rather than ignoring them, with a goal to fix the issues rather than ignore them.

ok, makes sense, i removed the clippy changes for now

src/buffer.rs Outdated Show resolved Hide resolved
src/backend/mod.rs Outdated Show resolved Hide resolved
@mindoodoo mindoodoo changed the title Lint and doc cleanup chore: lint and doc cleanup Jun 2, 2023
@mindoodoo
Copy link
Member

Wasn't exactly sure for the conventional commit (PR title) as this is both linting and a couple doc fixes so marked it as unscoped chore, maintainers feel free to change it to something more adequate if you have any ideas.

@mindoodoo mindoodoo added the documentation Improvements or additions to documentation label Jun 2, 2023
@codecov
Copy link

codecov bot commented Jun 2, 2023

Codecov Report

Merging #191 (808dfc5) into main (77067bd) will increase coverage by 0.02%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##             main     #191      +/-   ##
==========================================
+ Coverage   81.63%   81.66%   +0.02%     
==========================================
  Files          34       34              
  Lines        6574     6572       -2     
==========================================
  Hits         5367     5367              
+ Misses       1207     1205       -2     
Impacted Files Coverage Δ
src/backend/mod.rs 33.33% <0.00%> (+4.76%) ⬆️
src/buffer.rs 92.81% <ø> (ø)
src/terminal.rs 55.64% <ø> (ø)
src/layout.rs 85.25% <100.00%> (ø)
src/widgets/reflow.rs 98.80% <100.00%> (ø)

@joshka
Copy link
Member

joshka commented Jun 2, 2023

Wasn't exactly sure for the conventional commit (PR title) as this is both linting and a couple doc fixes so marked it as unscoped chore, maintainers feel free to change it to something more adequate if you have any ideas.

chore is fine. I made the changes that we both suggested (just used _n instead of a lint exception)

@nyurik
Copy link
Contributor Author

nyurik commented Jun 2, 2023

@joshka were the Cargo.toml changes intentional?

A few more minor cleanups, mostly in documentation
@nyurik
Copy link
Contributor Author

nyurik commented Jun 2, 2023

Ah, I see - you moved the indoc into dependencies because of the format usage, plus sorted them. Slightly outside of the initial scope, but sure, not a biggie :)

I removed the no-longer-needed comment about the underscore, and rebased on main

@orhun orhun merged commit 509d185 into ratatui-org:main Jun 2, 2023
8 of 10 checks passed
@nyurik nyurik deleted the docs branch June 2, 2023 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants