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

fix/53 Building a container image without a recipe name throws error #54

Merged

Conversation

deepu9
Copy link
Contributor

@deepu9 deepu9 commented Jun 16, 2024

This will let the users to run vib build with a recipe yml file in the folder. You can still provide the recipe name in the command, that's still the recommended way.

In addition to this fix, I have also added the following validation:

  1. file check to see whether the provided recipe path exists.
  2. extension check for either yml or yaml.

It's best to catch errors early in cmd than to leave to other packages.

It is also recommended to use errors.Is(...) instead of os.Is(...). Refer to the following golang issue

golang/go#41122

@deepu9 deepu9 marked this pull request as ready for review June 18, 2024 12:32
cmd/build.go Outdated Show resolved Hide resolved
@matbme
Copy link
Member

matbme commented Jun 18, 2024

Thanks for the contribution! Once you fix the typo above, could you please rebase everything in a single commit?

…e throws error

Check whether the provided recipe file exists and return error if not. Removed the line which overrides recipePath after being set.
@kbdharun kbdharun force-pushed the fix/53-cannot-build-container-without-recipe branch from a60fc67 to 224591e Compare June 18, 2024 17:35
@kbdharun kbdharun self-assigned this Jun 18, 2024
@kbdharun kbdharun requested a review from matbme June 18, 2024 17:35
Copy link
Member

@matbme matbme left a comment

Choose a reason for hiding this comment

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

LGTM

@matbme matbme merged commit 1485e58 into Vanilla-OS:main Jun 18, 2024
2 checks passed
Copy link
Member

@kbdharun kbdharun left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution. (Forgot to approve 😅 while rebasing)

@deepu9
Copy link
Contributor Author

deepu9 commented Jun 18, 2024

Sorry @matbme, I went to bed and didn't get a chance to fix it.

Thanks @kbdharun

@deepu9 deepu9 deleted the fix/53-cannot-build-container-without-recipe branch July 2, 2024 08:38
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