search

carvel-dev / vendir

Easy way to vendor portions of git repos, github releases, helm charts, docker image contents, etc. declaratively
https://carvel.dev/vendir
Apache License 2.0
269 stars 47 forks source link

fix: vendir sync --directory option error #286

Closed fritzduchardt closed 7 months ago

fritzduchardt commented 11 months ago

Fixes #277

kumaritanushree commented 9 months ago

LGTM

joaopapereira commented 8 months ago

@fritzduchardt looks like a test is failing, also, there are some conflicts. Do you mind checking that?

fritzduchardt commented 8 months ago

@fritzduchardt looks like a test is failing, also, there are some conflicts. Do you mind checking that?

Finally got that fixed. It was harder than expected, since the staging to final location copy logic was being affected.

Previously, the logic for dir-options rewrote the structure of vendir.yml. The staging copy logic relied on this new structure. I took out the structure rewrite, because thanks to my fix, the lock file is now written, even if sync is called with dir-options and we don't want its structure to deviate from the structure of vendir.yml.

fritzduchardt commented 8 months ago

@joaopapereira please have another look

joaopapereira commented 8 months ago

We are in a good spot, but I will let @kumaritanushree review it. The only ask for you @fritzduchardt is to get the commits signed so we can get rid of the failing DCO check

fritzduchardt commented 8 months ago

We are in a good spot, but I will let @kumaritanushree review it. The only ask for you @fritzduchardt is to get the commits signed so we can get rid of the failing DCO check

@joaopapereira I squashed them and signed them. Please have another look.

kumaritanushree commented 8 months ago

Looks good to me