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

refactor(layout): change "split"'s vars to be of known labeled values #396

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

hasezoey
Copy link
Contributor

This PR changes all values for the layout's split's vars value to be known and labeled, this changes the code to be more readable and defines the values in a common place, also removing a extra match arm

@codecov
Copy link

codecov bot commented Aug 12, 2023

Codecov Report

Merging #396 (8046763) into main (8c55158) will decrease coverage by 0.02%.
The diff coverage is 90.00%.

❗ Current head 8046763 differs from pull request most recent head 0fa4b77. Consider uploading reports for the commit 0fa4b77 to get more accurate results

@@            Coverage Diff             @@
##             main     #396      +/-   ##
==========================================
- Coverage   85.37%   85.36%   -0.02%     
==========================================
  Files          40       40              
  Lines        8875     8867       -8     
==========================================
- Hits         7577     7569       -8     
  Misses       1298     1298              
Files Changed Coverage Δ
src/layout.rs 88.97% <90.00%> (-0.22%) ⬇️

Copy link
Collaborator

@kdheepak kdheepak left a comment

Choose a reason for hiding this comment

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

LGTM

src/layout.rs Outdated Show resolved Hide resolved
src/layout.rs Outdated Show resolved Hide resolved
Removes some unnecessary code and makes the function more readable.
Instead of creating a temporary result and mutating it, we just create
the result directly from the list of changes.
@joshka
Copy link
Member

joshka commented Aug 14, 2023

Rebased on main, squashed and reworded the commit.

@joshka joshka enabled auto-merge August 14, 2023 23:16
@joshka joshka requested review from kdheepak and joshka August 14, 2023 23:17
@joshka joshka added this pull request to the merge queue Aug 14, 2023
Merged via the queue into ratatui:main with commit 5195099 Aug 14, 2023
@hasezoey hasezoey deleted the knownAttributes branch August 15, 2023 08:02
@joshka joshka added this to the v0.23.0 milestone Aug 21, 2023
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.

3 participants