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.

Sign up for GitHub

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

Jump to bottom

Refactor lazy sync code #340

Merged
merged 5 commits into from
Jan 10, 2024
Merged

Refactor lazy sync code #340

merged 5 commits into from
Jan 10, 2024

Conversation

Zebradil
Copy link
Member

@Zebradil Zebradil commented Dec 24, 2023 •
edited
Loading

This PR:

  • simplifies lazy sync flags handling
  • correctly handles os.Stat errors in some places
  • removes not needed functions
  • fixes spelling

For a reviewer: checking every commit separately may be easier, as the changes are grouped logically.

@kumaritanushree
Copy link
Contributor

kumaritanushree commented Dec 28, 2023 •
edited by 100mik
Loading

For a reviewer: checking every commit separately may be easier, as the changes are grouped logically.

@Zebradil this really makes easy in reviewing changes. Thanks
Changes looks good to me. Lets wait for @joaopapereira approval.

100mik
100mik approved these changes Jan 9, 2024
View reviewed changes
Copy link
Contributor

@100mik 100mik left a comment

Choose a reason for hiding this comment

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

These changes look good !
Appreciate you breaking it down into commits 🙌🏼

joaopapereira
joaopapereira requested changes Jan 9, 2024
View reviewed changes
Copy link
Member

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

I think it looks good, the only issue is the style of imports. Do you mind reverting that change?

pkg/vendir/cmd/sync.go Outdated
Comment on lines 15 to 18

ctlconf "carvel.dev/vendir/pkg/vendir/config"
ctldir "carvel.dev/vendir/pkg/vendir/directory"
ctlcache "carvel.dev/vendir/pkg/vendir/fetch/cache"
Copy link
Member

Choose a reason for hiding this comment

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

Stylewise in all the tools we do only 1 groups of imports, system and other, I think it would make sense for us to keep that consistency.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reverted this and in pkg/vendir/directory/directory.go too.

Those changes were done automatically by https://github.com/incu6us/goimports-reviser. I'll keep an eye on it next time.

joaopapereira
joaopapereira approved these changes Jan 10, 2024
View reviewed changes
Copy link
Member

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

LGTM

@joaopapereira joaopapereira merged commit 2763b6a into carvel-dev:develop Jan 10, 2024
4 checks passed
@Zebradil Zebradil deleted the lazy-sync-refactor branch January 10, 2024 20:29
renovate bot referenced this pull request in mykso/myks Jan 22, 2024
@renovate
fix(deps): update module carvel.dev/vendir to v0.39.0 (#225)
3a99a5d 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [carvel.dev/vendir](https://togithub.com/carvel-dev/vendir) |
`v0.38.0` -> `v0.39.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/carvel.dev%2fvendir/v0.39.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/carvel.dev%2fvendir/v0.39.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/carvel.dev%2fvendir/v0.38.0/v0.39.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/carvel.dev%2fvendir/v0.38.0/v0.39.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>carvel-dev/vendir (carvel.dev/vendir)</summary>

###
[`v0.39.0`](https://togithub.com/carvel-dev/vendir/releases/tag/v0.39.0)

[Compare
Source](https://togithub.com/carvel-dev/vendir/compare/v0.38.0...v0.39.0)

<details>

<summary><h2>Installation and signature verification</h2></summary>

##### Installation
##### By downloading binary from the release

For instance, if you are using Linux on an AMD64 architecture:

```shell

### Download the binary
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/vendir-linux-amd64

### Move the binary in to your PATH
mv vendir-linux-amd64 /usr/local/bin/vendir

### Make the binary executable
chmod +x /usr/local/bin/vendir
```

##### Via Homebrew (macOS or Linux)

```shell
$ brew tap carvel-dev/carvel
$ brew install vendir
$ vendir version  
```

##### Verify checksums file signature

Install cosign on your system
https://docs.sigstore.dev/system_config/installation/

The checksums file provided within the artifacts attached to this
release is signed using
[Cosign](https://docs.sigstore.dev/cosign/overview/) with GitHub OIDC.
To validate the signature of this file, run the following commands:

```shell

### Download the checksums file, certificate and signature
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt.pem
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt.sig

### Verify the checksums file
cosign verify-blob checksums.txt \
  --certificate checksums.txt.pem \
  --signature checksums.txt.sig \
  --certificate-identity-regexp=https://github.com/carvel-dev \
  --certificate-oidc-issuer=https://token.actions.githubusercontent.com
```

##### Verify binary integrity

To verify the integrity of the downloaded binary, you can utilize the
checksums file after having validated its signature.

```shell

### Verify the binary using the checksums file
sha256sum -c checksums.txt --ignore-missing
```

</details>

### ✨ What's new
* fix grammar in README by
@&#8203;vtrent[https://github.com/carvel-dev/vendir/pull/324](https://togithub.com/carvel-dev/vendir/pull/324)ll/324
* Added changes to sign artifacts by
@&#8203;kumaritanushr[https://github.com/carvel-dev/vendir/pull/339](https://togithub.com/carvel-dev/vendir/pull/339)ll/339
* Simplify gitignore and make sure all binaries are accounted for by
@&#8203;100m[https://github.com/carvel-dev/vendir/pull/351](https://togithub.com/carvel-dev/vendir/pull/351)ll/351
* PrivateKey with or without extra char as newline will be accepted by
@&#8203;kumaritanushr[https://github.com/carvel-dev/vendir/pull/349](https://togithub.com/carvel-dev/vendir/pull/349)ll/349
* Fix race condition when running multiple vendir from the same
directory by
@&#8203;Zebrad[https://github.com/carvel-dev/vendir/pull/345](https://togithub.com/carvel-dev/vendir/pull/345)ll/345
* Refactor lazy sync code by
@&#8203;Zebrad[https://github.com/carvel-dev/vendir/pull/340](https://togithub.com/carvel-dev/vendir/pull/340)ll/340
* Fix: updated setup cosign step in release process by
@&#8203;kumaritanushr[https://github.com/carvel-dev/vendir/pull/352](https://togithub.com/carvel-dev/vendir/pull/352)ll/352
* updated release to have installation and verification steps included
in release notes by
@&#8203;kumaritanushr[https://github.com/carvel-dev/vendir/pull/354](https://togithub.com/carvel-dev/vendir/pull/354)ll/354

#### New Contributors
* @&#8203;vtrenton made their first
contributi[https://github.com/carvel-dev/vendir/pull/324](https://togithub.com/carvel-dev/vendir/pull/324)ll/324
* @&#8203;100mik made their first
contributi[https://github.com/carvel-dev/vendir/pull/351](https://togithub.com/carvel-dev/vendir/pull/351)ll/351

**Full Changelog**:
carvel-dev/vendir@v0.38.0...v0.39.0

### 📂 Files Checksum

012531a2f1a2de8bc89f1623edfc40a7ac5aee421fe609085278fb9e287f1cdf
./vendir-linux-arm64
20b71cc25dc3fea31edf9667c92a05167f713935f854882159736443c2f7a0e6
./vendir-windows-amd64.exe
90ae82718c1072831f3097bdb031d5a897cc9f2f8334e2e1d7f35e35d0abd84f
./vendir-darwin-amd64
91ecf04ad5cdfa0f8839dc1430da7a4da665f7cb88c64c0c72202f6db261e651
./vendir-darwin-arm64
feb2836153508adfb6fd33c127e466c9ce26577678e93a252be2fec445f4501f
./vendir-linux-amd64

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log [here](https://developer.mend.io/github/mykso/myks).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMzUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEzNS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot referenced this pull request in mykso/myks Jan 22, 2024
@renovate
chore(deps): update dependency carvel-dev/vendir to v0.39.0 (#224)
492fb54 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [carvel-dev/vendir](https://togithub.com/carvel-dev/vendir) | minor |
`v0.38.0` -> `v0.39.0` |

---

### Release Notes

<details>
<summary>carvel-dev/vendir (carvel-dev/vendir)</summary>

###
[`v0.39.0`](https://togithub.com/carvel-dev/vendir/releases/tag/v0.39.0)

[Compare
Source](https://togithub.com/carvel-dev/vendir/compare/v0.38.0...v0.39.0)

<details>

<summary><h2>Installation and signature verification</h2></summary>

##### Installation
##### By downloading binary from the release

For instance, if you are using Linux on an AMD64 architecture:

```shell

### Download the binary
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/vendir-linux-amd64

### Move the binary in to your PATH
mv vendir-linux-amd64 /usr/local/bin/vendir

### Make the binary executable
chmod +x /usr/local/bin/vendir
```

##### Via Homebrew (macOS or Linux)

```shell
$ brew tap carvel-dev/carvel
$ brew install vendir
$ vendir version  
```

##### Verify checksums file signature

Install cosign on your system
https://docs.sigstore.dev/system_config/installation/

The checksums file provided within the artifacts attached to this
release is signed using
[Cosign](https://docs.sigstore.dev/cosign/overview/) with GitHub OIDC.
To validate the signature of this file, run the following commands:

```shell

### Download the checksums file, certificate and signature
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt.pem
curl -LO https://github.com/carvel-dev/vendir/releases/download/v0.39.0/checksums.txt.sig

### Verify the checksums file
cosign verify-blob checksums.txt \
  --certificate checksums.txt.pem \
  --signature checksums.txt.sig \
  --certificate-identity-regexp=https://github.com/carvel-dev \
  --certificate-oidc-issuer=https://token.actions.githubusercontent.com
```

##### Verify binary integrity

To verify the integrity of the downloaded binary, you can utilize the
checksums file after having validated its signature.

```shell

### Verify the binary using the checksums file
sha256sum -c checksums.txt --ignore-missing
```

</details>

### ✨ What's new
* fix grammar in README by
@&#8203;vtrent[https://github.com/carvel-dev/vendir/pull/324](https://togithub.com/carvel-dev/vendir/pull/324)ll/324
* Added changes to sign artifacts by
@&#8203;kumaritanushr[https://github.com/carvel-dev/vendir/pull/339](https://togithub.com/carvel-dev/vendir/pull/339)ll/339
* Simplify gitignore and make sure all binaries are accounted for by
@&#8203;100m[https://github.com/carvel-dev/vendir/pull/351](https://togithub.com/carvel-dev/vendir/pull/351)ll/351
* PrivateKey with or without extra char as newline will be accepted by
@&#8203;kumaritanushr[https://github.com/carvel-dev/vendir/pull/349](https://togithub.com/carvel-dev/vendir/pull/349)ll/349
* Fix race condition when running multiple vendir from the same
directory by
@&#8203;Zebrad[https://github.com/carvel-dev/vendir/pull/345](https://togithub.com/carvel-dev/vendir/pull/345)ll/345
* Refactor lazy sync code by
@&#8203;Zebrad[https://github.com/carvel-dev/vendir/pull/340](https://togithub.com/carvel-dev/vendir/pull/340)ll/340
* Fix: updated setup cosign step in release process by
@&#8203;kumaritanushr[https://github.com/carvel-dev/vendir/pull/352](https://togithub.com/carvel-dev/vendir/pull/352)ll/352
* updated release to have installation and verification steps included
in release notes by
@&#8203;kumaritanushr[https://github.com/carvel-dev/vendir/pull/354](https://togithub.com/carvel-dev/vendir/pull/354)ll/354

#### New Contributors
* @&#8203;vtrenton made their first
contributi[https://github.com/carvel-dev/vendir/pull/324](https://togithub.com/carvel-dev/vendir/pull/324)ll/324
* @&#8203;100mik made their first
contributi[https://github.com/carvel-dev/vendir/pull/351](https://togithub.com/carvel-dev/vendir/pull/351)ll/351

**Full Changelog**:
carvel-dev/vendir@v0.38.0...v0.39.0

### 📂 Files Checksum

012531a2f1a2de8bc89f1623edfc40a7ac5aee421fe609085278fb9e287f1cdf
./vendir-linux-arm64
20b71cc25dc3fea31edf9667c92a05167f713935f854882159736443c2f7a0e6
./vendir-windows-amd64.exe
90ae82718c1072831f3097bdb031d5a897cc9f2f8334e2e1d7f35e35d0abd84f
./vendir-darwin-amd64
91ecf04ad5cdfa0f8839dc1430da7a4da665f7cb88c64c0c72202f6db261e651
./vendir-darwin-arm64
feb2836153508adfb6fd33c127e466c9ce26577678e93a252be2fec445f4501f
./vendir-linux-amd64

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log [here](https://developer.mend.io/github/mykso/myks).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMzUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEzNS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@Zebradil Zebradil mentioned this pull request May 3, 2024
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@100mik 100mik 100mik approved these changes

@joaopapereira joaopapereira joaopapereira approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Carvel
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Zebradil @kumaritanushree @joaopapereira @100mik