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

Bug: Fetch release now depends on clean. It fails when the sync folde… #317

Merged
merged 2 commits into from
Feb 14, 2022

Conversation

AustinAbro321
Copy link
Contributor

Was going through the zarf big bang example and noticed this! Here is an example of what happens when you try to run the make fetch-release while the sync folder isn't created.
zarf_make_fetch_release_error

@YrrepNoj
Copy link
Contributor

@AustinAbro321, thanks for pushing this. I feel like I remember running into this the first time I tried to build and then just forgot about it since I already had the sync directory afterwards.

What do you think about adding @mkdir -p sync in the steps instead of executing the clean recipe? I worry that someone might lose all their packages just from trying to make sure they have the latest binary release. To be honest, I'm perfectly fine keeping it like this, just wanted to get your thoughts.

We also are trying to keep the commits in this repo signed and verified. Here are some of GitHubs docs on doing that. Let me know if you have any questions on this because I know these instructions aren't always the most clear. https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

@YrrepNoj YrrepNoj requested review from RothAndrew and YrrepNoj and removed request for RothAndrew February 11, 2022 20:23
@RothAndrew
Copy link
Contributor

I agree with Jon. I'd rather add mkdir -p sync to the target than add clean as a dependency.

@AustinAbro321
Copy link
Contributor Author

Yeah that makes sense. I hadn't realized that more gets stored in sync than just the releases. I'll configure my git to sign commits and push that change up

@RothAndrew
Copy link
Contributor

You'll need to squash with the last commit and force push (or make a new branch). We have GitHub configured to require ALL commits in a PR to be signed before it is able to merge.

@AustinAbro321
Copy link
Contributor Author

Change is made and commits are squashed and verified. And I learned about git rebase along the way!

@YrrepNoj
Copy link
Contributor

/test all

@jeff-mccoy
Copy link
Contributor

/test all

@jeff-mccoy jeff-mccoy merged commit 95b8cbb into master Feb 14, 2022
@jeff-mccoy jeff-mccoy deleted the make-sync-dir-bug branch February 14, 2022 20:04
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.

4 participants