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

feat: add YAML and JSON support for bundles #128

Merged

Conversation

b4nst
Copy link
Contributor

@b4nst b4nst commented Jun 21, 2023

This little addition add YAML support for bundle file. It can be useful for multiple purposes, like using sops encrypted files until we address #74.

I'm also referencing cue-lang/cue/discussions/2452 in case a better way of including Orphan files exists.

Copy link
Owner

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

Hey @b4nst we have similar feature for values, where we allow both YAML and JSON. I think we need to support JSON as well for consistency.

Also a test with a bundle made out of 3 files CUE, YAML and JSON would be good.

@b4nst
Copy link
Contributor Author

b4nst commented Jun 29, 2023

Hey @stefanprodan, just added JSON support and a test case with the combo of all file types.

@stefanprodan
Copy link
Owner

Thanks @b4nst can you please rebase with main, solve conflicts and force push to unblock CI.

@stefanprodan stefanprodan added enhancement New feature or request area/bundles Timoni's Bundle related issues and pull requests labels Jun 29, 2023
@stefanprodan stefanprodan changed the title feat: add yaml support for bundles feat: add YAML and JSON support for bundles Jun 29, 2023
@b4nst
Copy link
Contributor Author

b4nst commented Jun 29, 2023

Of course! Sorry I went a bit fast on the push 😅

@b4nst b4nst force-pushed the feat/support-yaml-bundle-files branch from 5edc239 to 06298a6 Compare June 29, 2023 15:45
@b4nst
Copy link
Contributor Author

b4nst commented Jun 29, 2023

Ok rebased, seems there's an issue with the Copy and YAML files during workspace creation introduced with the injection system (break the content of the file). I'll give it a look

@stefanprodan
Copy link
Owner

@b4nst I suggest we put in the workspace not the jsons and yamls but their cue value.

@b4nst
Copy link
Contributor Author

b4nst commented Jun 30, 2023

Yup, that's what I started (moving the extract part into the workspace function). Incoming today

@b4nst b4nst force-pushed the feat/support-yaml-bundle-files branch from 06298a6 to 8894bc2 Compare July 6, 2023 10:10
@b4nst b4nst force-pushed the feat/support-yaml-bundle-files branch 3 times, most recently from b8f7959 to a40a1fb Compare July 6, 2023 13:35
@b4nst b4nst force-pushed the feat/support-yaml-bundle-files branch from a40a1fb to 3b6568b Compare July 6, 2023 15:18
@b4nst b4nst force-pushed the feat/support-yaml-bundle-files branch from 3b6568b to 6c51f9e Compare July 6, 2023 15:45
Copy link
Contributor

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

This looks good to me 💯

@b4nst
Copy link
Contributor Author

b4nst commented Jul 7, 2023

Is it good to merge @stefanprodan?

Copy link
Owner

@stefanprodan stefanprodan 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 @b4nst 🥇

@stefanprodan stefanprodan merged commit 30b3bb4 into stefanprodan:main Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bundles Timoni's Bundle related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants