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.

Sign up for GitHub

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

Jump to bottom

✨ Add JSON schema for Vald Helm Chart #365

Merged
merged 10 commits into from
May 27, 2020
Merged

✨ Add JSON schema for Vald Helm Chart #365

merged 10 commits into from
May 27, 2020

Conversation

rinx
Copy link
Contributor

@rinx rinx commented May 7, 2020 •
edited
Loading

Signed-off-by: Rintaro Okamura rintaro.okamura@gmail.com

Description:

Add JSON schema file to Vald Helm Chart.
To achieve this, hack script is added in this PR.

operator sdk version is downgraded because Docker image of v0.17.1 is not uploaded yet. 😭

Related Issue:

How Has This Been Tested?:

Environment:

  • Golang Version: 1.14
  • Docker Version: 19.03.5
  • Kubernetes Version: 1.17.3
  • NGT Version: 1.9.1

Types of changes:

  • Bug fix [type/bug]
  • New feature [type/feature]
  • Add tests [type/test]
  • Security related changes [type/security]
  • Add documents [type/documentation]
  • Refactoring [type/refactoring]
  • Update dependencies [type/dependency]
  • Update benchmarks and performances [type/bench]
  • Update CI [type/ci]

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Checklist:

  • I have read the CONTRIBUTING document.
  • I have checked open Pull Requests for the similar feature or fixes?
  • I have added tests and benchmarks to cover my changes.
  • I have ensured all new and existing tests passed.
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly.

@pull-assistant
Copy link

pull-assistant bot commented May 7, 2020 •
edited
Loading

Score: 0.93

Best reviewed: commit by commit

Optimal code review plan (2 warnings)

     ✨ Add JSON schema for Vald Helm Chart

♻️ Rename schema

hack/helm/schema/gen/main.go 88% changes removed in ⚡ Add validation...

⚡ Add validation terms based on json-schema draft07

hack/helm/schema/gen/main.go 43% changes removed in 🚨 Fix...

     📝 Add more validations

     ✨ Add more schemas / Add anchor + aliases

     ✨ Add schema for vald-helm-operator

     🍱 Add schema.jsons & update CI

     🚨 Fix lint warning

     ⬇️ downgrade operator sdk

     💚 update helm-lint

Powered by Pull Assistant. Last update cddb7d5 ... 63fbabe. Read the comment docs.

@rinx rinx force-pushed the documentation/helm-chart/add-json-schema branch 3 times, most recently from 555dabd to 0c3a7f0 Compare May 12, 2020 06:48
@rinx rinx force-pushed the documentation/helm-chart/add-json-schema branch 2 times, most recently from eae576b to 5c33872 Compare May 19, 2020 09:32
github-actions[bot]
github-actions bot reviewed May 19, 2020
View reviewed changes
hack/helm/schema/gen/main.go Show resolved Hide resolved
hack/helm/schema/gen/main.go Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go
}

func genJSONSchema(path string) error {
f, err := os.Open(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
G304: Potential file inclusion via variable (gosec)

hack/helm/schema/gen/main.go
}

func main() {
if len(os.Args) < 2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
mnd: Magic number: 2, in detected (gomnd)

hack/helm/schema/gen/main.go
l := ls[0]
switch l.Type {
case objectType:
if len(ls) <= 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
mnd: Magic number: 1, in detected (gomnd)

@rinx rinx force-pushed the documentation/helm-chart/add-json-schema branch from dec7557 to 5ffad08 Compare May 25, 2020 06:40
github-actions[bot]
github-actions bot reviewed May 25, 2020
View reviewed changes
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go Outdated Show resolved Hide resolved
hack/helm/schema/gen/main.go
aliases map[string]Schema
)

type SchemaBase struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
struct of size 192 bytes could be of size 176 bytes (maligned)

@rinx rinx marked this pull request as ready for review May 25, 2020 07:16
@rinx rinx requested review from hlts2 and kpango May 25, 2020 07:20
@rinx rinx force-pushed the documentation/helm-chart/add-json-schema branch from 1dfe372 to 18d782c Compare May 25, 2020 07:23
@kpango
Copy link
Collaborator

kpango commented May 26, 2020

/rebase
/format

@vdaas-ci
Copy link
Collaborator

[REBASE] Rebase triggered by kpango for branch: documentation/helm-chart/add-json-schema

@vdaas-ci
Copy link
Collaborator

[REBASE] Failed to rebase.

rinx added 7 commits May 26, 2020 20:47
@rinx
✨ Add JSON schema for Vald Helm Chart
cddb7d5 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx
♻️ Rename schema
6ab8602 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx
⚡ Add validation terms based on json-schema draft07
71aefd6 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx
📝 Add more validations
44377b3 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx
✨ Add more schemas / Add anchor + aliases
0ae18c2 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx
✨ Add schema for vald-helm-operator
5372d7d 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx
🍱 Add schema.jsons & update CI
a8b7af0 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
rinx added 3 commits May 26, 2020 20:47
@rinx
🚨 Fix lint warning
fc3ecc2 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx
⬇️ downgrade operator sdk
2619f67 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx
💚 update helm-lint
63fbabe 
Signed-off-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@rinx rinx force-pushed the documentation/helm-chart/add-json-schema branch from 18d782c to 63fbabe Compare May 26, 2020 11:50
kpango
kpango approved these changes May 27, 2020
View reviewed changes
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

LGTM

@kpango kpango merged commit 814480c into master May 27, 2020
@kpango kpango deleted the documentation/helm-chart/add-json-schema branch May 27, 2020 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions GitHub Actions GitHub Actions left review comments

@kpango kpango kpango approved these changes

@hlts2 hlts2 Awaiting requested review from hlts2

Assignees
No one assigned
Labels
area/helm area/makefile priority/low size/L team/sre SRE team type/ci type/documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rinx @kpango @vdaas-ci