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: implement knowledge v1beta2 #193

Merged
merged 1 commit into from
Feb 8, 2024
Merged

feat: implement knowledge v1beta2 #193

merged 1 commit into from
Feb 8, 2024

Conversation

cowan-macady
Copy link
Contributor

@cowan-macady cowan-macady commented Jan 24, 2024

BREAKING CHANGE: ENG-2778

  • add knowledge v1beta2
  • add objects v1beta2
  • update helper
  • remove KG config node
  • change tests

@cowan-macady cowan-macady requested a review from a team as a code owner January 24, 2024 15:51
@cowan-macady cowan-macady requested a review from maaland January 24, 2024 15:51
Copy link

codecov bot commented Jan 24, 2024

Codecov Report

Attention: 49 lines in your changes are missing coverage. Please review.

Comparison is base (ea3b16a) 72.01% compared to head (a526bba) 70.02%.

Files Patch % Lines
knowledge/knowledge_read.go 57.01% 49 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
- Coverage   72.01%   70.02%   -2.00%     
==========================================
  Files          39       39              
  Lines        2326     2345      +19     
==========================================
- Hits         1675     1642      -33     
- Misses        596      650      +54     
+ Partials       55       53       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@maaland maaland left a comment

Choose a reason for hiding this comment

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

Mostly good, just some comments.

I see you're replacing the old version with the new version. Can you first check with @yetizer / @bakkelim that that's okay for them? It might cause issues if they need a fix to something that comes in a later SDK version, but can't update yet because they're not ready to move to IKG API v2

examples/knowledge/cmd/read.go Outdated Show resolved Hide resolved
knowledge/example_test.go Outdated Show resolved Hide resolved
knowledge/example_test.go Outdated Show resolved Hide resolved
knowledge/example_test.go Outdated Show resolved Hide resolved
knowledge/example_test.go Outdated Show resolved Hide resolved
knowledge/example_test.go Outdated Show resolved Hide resolved
knowledge/example_test.go Outdated Show resolved Hide resolved
knowledge/knowledge_read.go Outdated Show resolved Hide resolved
knowledge/knowledge_read.go Outdated Show resolved Hide resolved
@cowan-macady cowan-macady requested review from maaland, a team and arxeiss and removed request for arxeiss January 25, 2024 07:41
examples/knowledge/cmd/read.go Outdated Show resolved Hide resolved
knowledge/knowledge_read.go Outdated Show resolved Hide resolved
knowledge/knowledge_read.go Show resolved Hide resolved
knowledge/knowledge_read.go Outdated Show resolved Hide resolved
@cowan-macady cowan-macady force-pushed the ikg-api-v2 branch 3 times, most recently from 4f8a046 to f1acfc8 Compare February 6, 2024 14:24
@cowan-macady
Copy link
Contributor Author

cowan-macady commented Feb 6, 2024

To be continued with modifications from objects v2 in new PR

@cowan-macady cowan-macady requested a review from maaland February 6, 2024 14:28
examples/knowledge/cmd/property.go Outdated Show resolved Hide resolved
knowledge/knowledge_read.go Outdated Show resolved Hide resolved
Copy link
Contributor

@maaland maaland left a comment

Choose a reason for hiding this comment

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

Looks fine, just add more possible query values

knowledge/knowledge_read.go Outdated Show resolved Hide resolved
@cowan-macady cowan-macady merged commit 4d657e9 into master Feb 8, 2024
10 checks passed
@cowan-macady cowan-macady deleted the ikg-api-v2 branch February 8, 2024 09:23
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