Skip to content

Conversation

@frelon
Copy link
Contributor

@frelon frelon commented Jan 25, 2024

This includes the update to vfs4.

Fixes #1911, #1913

@frelon frelon requested a review from a team as a code owner January 25, 2024 07:26
@frelon frelon force-pushed the fork-yip branch 2 times, most recently from 0d5eb40 to 17c99c0 Compare January 25, 2024 07:38
@frelon frelon linked an issue Jan 25, 2024 that may be closed by this pull request
Signed-off-by: Fredrik Lönnegren <[email protected]>
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cfcae7c) 73.85% compared to head (cfc1329) 73.83%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1916      +/-   ##
==========================================
- Coverage   73.85%   73.83%   -0.02%     
==========================================
  Files          71       71              
  Lines        7779     7759      -20     
==========================================
- Hits         5745     5729      -16     
+ Misses       1613     1611       -2     
+ Partials      421      419       -2     

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

Copy link
Contributor

@davidcassany davidcassany left a comment

Choose a reason for hiding this comment

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

LGTM

@davidcassany davidcassany merged commit 324583f into rancher:main Jan 25, 2024
@frelon frelon deleted the fork-yip branch January 25, 2024 10:11
@kkaempf
Copy link
Contributor

kkaempf commented Jan 25, 2024

This shouldn't have been merged.

It has a misleading title. 😞

@frelon
Copy link
Contributor Author

frelon commented Jan 25, 2024

This shouldn't have been merged.

It has a misleading title. 😞

I'm confused, what is misleading about the title?

@kkaempf
Copy link
Contributor

kkaempf commented Jan 25, 2024

This shouldn't have been merged.
It has a misleading title. 😞

I'm confused, what is misleading about the title?

The title just references one change ("Use rancher/yip v1.4.8") but the PR changes two things (update of yip, change to rancher/yip). This should've been two separate PRs imho.

@frelon
Copy link
Contributor Author

frelon commented Jan 25, 2024

This shouldn't have been merged.

It has a misleading title. 😞

I'm confused, what is misleading about the title?

The title just references one change ("Use rancher/yip v1.4.8") but the PR changes two things (update of yip, change to rancher/yip). This should've been two separate PRs imho.

Technically the title references both changes..

I agree it could have been two commits, but spending time splitting these relatively small bumps into several feels nitpicky to me (especially if the commits are later squashed...).

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.

Update yip Switch yip to rancher/yip

4 participants