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 json schema #329

Merged
merged 10 commits into from
Nov 9, 2023
Merged

Feat json schema #329

merged 10 commits into from
Nov 9, 2023

Conversation

roytev
Copy link
Contributor

@roytev roytev commented Nov 4, 2023

what

  • Added a json schema for the charts values
  • Added missing customizations to the values.yaml
  • Added missing customizations to the Readme.md table

why

  • make sure that if we change something in the values yaml we are not breaking something.
  • when user install or upgrade it gets more verbose information if he put something wrong in the values.
  • also when user make typo the additionalproperties:false will let him know that the attribute is not supported.
  • when using intelliJ and i presume other IDE's its much easier to make changes as we are seeing the description from the schema.
  • its a preparation of an effort of mine to make this chart more robust and maintainable. i also want to add helm unit tests

tests

  • ran helm lint with all possible values.yaml attribute

references

@roytev roytev requested a review from a team as a code owner November 4, 2023 18:33
@jamengual
Copy link
Contributor

could you bump the version please?

@roytev
Copy link
Contributor Author

roytev commented Nov 5, 2023

@jamengual Thanks done!

Copy link
Member

@GMartinez-Sisti GMartinez-Sisti left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @roytev! Left some comments/suggestions.

charts/atlantis/Chart.yaml Outdated Show resolved Hide resolved
charts/atlantis/values.yaml Outdated Show resolved Hide resolved
charts/atlantis/values.yaml Outdated Show resolved Hide resolved
@roytev
Copy link
Contributor Author

roytev commented Nov 5, 2023

@GMartinez-Sisti Thanks! ive made the requested changes

Copy link
Member

@GMartinez-Sisti GMartinez-Sisti left a comment

Choose a reason for hiding this comment

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

LGTM 😄

jamengual
jamengual previously approved these changes Nov 9, 2023
@jamengual jamengual merged commit 24e58d0 into runatlantis:main Nov 9, 2023
2 checks passed
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