Skip to content

Rsdk 2582 new vision service protos #261

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
Merged
merged 8 commits into from
Apr 25, 2023
Merged

Rsdk 2582 new vision service protos #261

merged 8 commits into from
Apr 25, 2023

Conversation

purplenicole730
Copy link
Member

@purplenicole730 purplenicole730 commented Apr 19, 2023 •
edited
Loading

RSDK-2582

Changes in Vision Service introduced before scheduled release:

  • generated protos locally that included many deletions of functions
  • delete related functions in vision service client
  • delete related tests and mocks

@purplenicole730 purplenicole730 requested a review from a team as a code owner April 19, 2023 21:08
purplenicole730
purplenicole730 commented Apr 19, 2023
View reviewed changes
@purplenicole730 purplenicole730 requested a review from cheukt April 19, 2023 21:12
cheukt
cheukt requested changes Apr 20, 2023
View reviewed changes
Copy link
Member

@cheukt cheukt left a comment

Choose a reason for hiding this comment

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

if a parameter is no longer needed in the proto, we can get rid of it from the wrapper as well

@purplenicole730 purplenicole730 requested a review from cheukt April 20, 2023 20:12
cheukt
cheukt approved these changes Apr 21, 2023
View reviewed changes
Copy link
Member

@cheukt cheukt left a comment

Choose a reason for hiding this comment

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

LGTM but remember to remove VisModelType and VisModelConfig as well

stuqdog
stuqdog approved these changes Apr 21, 2023
View reviewed changes
Copy link
Member

@stuqdog stuqdog left a comment

Choose a reason for hiding this comment

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

lgtm!

bhaney
bhaney reviewed Apr 21, 2023
View reviewed changes
@purplenicole730 purplenicole730 merged commit d275209 into main Apr 25, 2023
@purplenicole730 purplenicole730 deleted the RSDK-2582-new-vision-service-protos branch April 25, 2023 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhaney bhaney bhaney left review comments

@stuqdog stuqdog stuqdog approved these changes

@benjirewis benjirewis benjirewis approved these changes

@cheukt cheukt cheukt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@purplenicole730 @bhaney @stuqdog @benjirewis @cheukt