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

[patch] Documentation: add concept section to the architecture document #438

Merged
merged 4 commits into from
Jun 9, 2020
Merged

[patch] Documentation: add concept section to the architecture document #438

merged 4 commits into from
Jun 9, 2020

Conversation

kevindiu
Copy link
Contributor

@kevindiu kevindiu commented Jun 4, 2020 •
edited
Loading

Description:

This document add a concept section to the architecture document to give an overall concept of Vald before deep diving into details.

Related Issue:

How Has This Been Tested?:

Environment:

  • Golang Version: 1.14.3
  • Docker Version: 19.03.8
  • Kubernetes Version: 1.18.2
  • NGT Version: 1.11.5

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 Jun 4, 2020 •
edited
Loading

Score: 0.75

Best reviewed: commit by commit

Optimal code review plan (1 warning)

     add_concept_section

fix grammar

docs/overview/architecture.md 57% changes removed in Apply suggestions fr...

     Apply suggestions from code review

     Apply suggestions from code review

Powered by Pull Assistant. Last update 7044095 ... e2afede. Read the comment docs.

@kevindiu kevindiu requested a review from hlts2 June 8, 2020 06:14
hlts2
hlts2 reviewed Jun 8, 2020
View reviewed changes
docs/overview/architecture.md Outdated Show resolved Hide resolved
hlts2
hlts2 previously approved these changes Jun 8, 2020
View reviewed changes
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM

@kevindiu kevindiu marked this pull request as ready for review June 8, 2020 06:28
@kevindiu kevindiu requested a review from vankichi June 8, 2020 06:28
@kevindiu kevindiu changed the title [Draft] Documentation: add concept section to the architecture document Documentation: add concept section to the architecture document Jun 8, 2020
vankichi
vankichi previously approved these changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@vankichi vankichi left a comment

Choose a reason for hiding this comment

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

LGTM
/approve
/rebase
/approve

@vankichi vankichi requested review from kpango, rinx and kmrmt June 8, 2020 07:10
@vankichi
Copy link
Contributor

vankichi commented Jun 8, 2020 •
edited
Loading

@kpango @rinx @kmrmt
We (SET team) considered we should describe about Vald's basic concepts (e.g. cloud native, apply micro service architecture, not deep dive each component) before showing Vald's overall architecture images / data flows to users because it will help for new users.
So, we added Vald's basic concepts section at the top of architecture document.
We'd like to get authors' review whether added document is correct or not.
Could you please review by 6/10? And, we appreciate you if you can review ASAP.
thank you for your cooperation 🙏

@kpango
Copy link
Collaborator

kpango commented Jun 8, 2020 •
edited
Loading

@vankichi thank you for your team's great job, this week I'm so busy could you please wait until 6/10?

@vankichi
Copy link
Contributor

vankichi commented Jun 8, 2020 •
edited
Loading

@kpango
Sure ! But please remain that we'd like to publish this update by 6/12 (Fri.) because of KubeFest Tokyo 2020.
(I just updated the deadline)

rinx
rinx suggested changes Jun 8, 2020
View reviewed changes
docs/overview/architecture.md Outdated Show resolved Hide resolved
docs/overview/architecture.md Outdated Show resolved Hide resolved
docs/overview/architecture.md Outdated Show resolved Hide resolved
docs/overview/architecture.md Outdated Show resolved Hide resolved
docs/overview/architecture.md Outdated Show resolved Hide resolved
docs/overview/architecture.md Outdated Show resolved Hide resolved
@vankichi
Copy link
Contributor

vankichi commented Jun 9, 2020

/rebase

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 9, 2020

[REBASE] Rebase triggered by vankichi for branch: documentation/add_concept

@vdaas-ci vdaas-ci dismissed stale reviews from vankichi and hlts2 via 68338e8 June 9, 2020 01:22
kevindiu
kevindiu commented Jun 9, 2020
View reviewed changes
docs/overview/architecture.md Outdated Show resolved Hide resolved
docs/overview/architecture.md Outdated Show resolved Hide resolved
docs/overview/architecture.md Outdated Show resolved Hide resolved
@kevindiu kevindiu requested a review from rinx June 9, 2020 08:43
kpango
kpango previously approved these changes Jun 9, 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

kevindiu and others added 3 commits June 9, 2020 18:46
@kevindiu @vankichi
add_concept_section
7044095
@kevindiu @vankichi
fix grammar
be6cfa5
@kevindiu @hlts2 @vankichi
Apply suggestions from code review
6f182c3 
Co-authored-by: Hiroto Funakoshi <hiroto.funakoshi.hiroto@gmail.com>
@kevindiu @rinx @vankichi
Apply suggestions from code review
e2afede 
Co-authored-by: Rintaro Okamura <rintaro.okamura@gmail.com>
@kpango kpango changed the title Documentation: add concept section to the architecture document [patch] Documentation: add concept section to the architecture document Jun 9, 2020
@kpango
Copy link
Collaborator

kpango commented Jun 9, 2020

/format
/changelog
/approve

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 9, 2020

[FORMAT] Updating license headers and formatting go codes triggered by kpango.

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 9, 2020

[FORMAT] Failed to format.

@kpango
Copy link
Collaborator

kpango commented Jun 9, 2020

/changelog
/approve

vdaas-ci
vdaas-ci approved these changes Jun 9, 2020
View reviewed changes
Copy link
Collaborator

@vdaas-ci vdaas-ci left a comment

Choose a reason for hiding this comment

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

[APPROVED] This PR is approved by kpango.

@kpango kpango merged commit 36fb801 into master Jun 9, 2020
@kpango kpango deleted the documentation/add_concept branch June 9, 2020 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vdaas-ci vdaas-ci vdaas-ci approved these changes

@hlts2 hlts2 hlts2 left review comments

@vankichi vankichi vankichi left review comments

@kpango kpango kpango left review comments

@kmrmt kmrmt Awaiting requested review from kmrmt

@rinx rinx Awaiting requested review from rinx

Assignees
No one assigned
Labels
priority/low size/M status/author-review type/documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kevindiu @vankichi @kpango @vdaas-ci @rinx @hlts2