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(conditions): Simplify conditions #10

Merged
merged 2 commits into from
Feb 4, 2024
Merged

Conversation

yyvess
Copy link
Contributor

@yyvess yyvess commented Jan 28, 2024

I simplify code by removing if #component.

ServiceAccount & PodDisruptionBudget are now generic.

About Role & RoleBinding, for me it's preferable to define types as to use conditions.

About deployment, I thinks we can continue to simplify it by moving common spec field to #Deployment

What do you thinks ?

@yyvess yyvess force-pushed the feat/conditions branch 6 times, most recently from afd1821 to ebc3687 Compare January 28, 2024 08:45
@Nalum
Copy link
Owner

Nalum commented Jan 28, 2024

This is looking really good @yyvess, appreciate you taking the time to dig in. I'll have a proper look at this tomorrow but agree about simplifying things where possible.

@yyvess
Copy link
Contributor Author

yyvess commented Jan 28, 2024

@Nalum Take your time to review it. Refactoring it help me to learn Cue ;-)

I remove around 250 lines of configuration, I test it with default value and some others, result still same.
But if you have a more complexe values file, please compare if the Yaml result still the same for you too.

@yyvess yyvess force-pushed the feat/conditions branch 4 times, most recently from ca96f47 to 228ba43 Compare January 28, 2024 13:10
@yyvess yyvess marked this pull request as ready for review January 29, 2024 09:23
@Nalum Nalum merged commit 15e54c4 into Nalum:main Feb 4, 2024
5 checks passed
@Nalum
Copy link
Owner

Nalum commented Feb 4, 2024

Thank you @yyvess for this awesome PR! ❤️

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.

2 participants