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(knowledge-base): follow the convention #349

Merged
merged 10 commits into from
May 30, 2024
Merged

feat(knowledge-base): follow the convention #349

merged 10 commits into from
May 30, 2024

Conversation

Yougigun
Copy link
Contributor

@Yougigun Yougigun commented May 29, 2024

Because

of consistency with other services

This commit

follows the convention

Note:
this PR is only for POC, and there are some known issues will be refactored soon.

@Yougigun Yougigun changed the title feat(artifact): add fieled 2 feat(artifact): add inuse fieled May 29, 2024
Copy link
Member

@donch1989 donch1989 left a comment

Choose a reason for hiding this comment

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

  1. Besides the above comments, for consistency, we should use ListKnowledgeBases and GetKnowledgeBase for our gRPC endpoint naming.
  2. How do we handle getting KnowledgeBase from different owners?
  3. There are some typos in the PR title and message.

artifact/artifact/v1alpha/artifact.proto Outdated Show resolved Hide resolved
artifact/artifact/v1alpha/artifact.proto Outdated Show resolved Hide resolved
artifact/artifact/v1alpha/artifact.proto Outdated Show resolved Hide resolved
@Yougigun Yougigun changed the title feat(artifact): add inuse fieled feat(artifact): follow the convention May 30, 2024
@Yougigun Yougigun changed the title feat(artifact): follow the convention feat(knowledge-base): follow the convention May 30, 2024
@donch1989
Copy link
Member

Hi @Yougigun ,
There are still some design differences compared to other backends. Can you refer to the pipeline or model API design as an example? Keeping the same API design will help clients integrate more easily.

@Yougigun
Copy link
Contributor Author

Yougigun commented May 30, 2024

Hi @Yougigun , There are still some design differences compared to other backends. Can you refer to the pipeline or model API design as an example? Keeping the same API design will help clients integrate more easily.

@donch1989 Thank you for pointing out.
That's right. I had not noticed the design consistency with other services. Due to mvp testing purposes, I will refactor it in the next version(beta) with @thewbuk

@Yougigun Yougigun requested a review from donch1989 May 30, 2024 04:16
@donch1989
Copy link
Member

Hi, @Yougigun
Please write a note in commit message to show that this PR is only for POC, and there are some known issues will be refactored soon.

@Yougigun
Copy link
Contributor Author

Hi, @Yougigun Please write a note in commit message to show that this PR is only for POC, and there are some known issues will be refactored soon.

done.

@Yougigun Yougigun merged commit e582423 into main May 30, 2024
2 checks passed
@Yougigun Yougigun deleted the gary/kb-proto-1 branch May 30, 2024 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: 👋 Done
Development

Successfully merging this pull request may close these issues.

3 participants