Skip to content

Conversation

@DanG100
Copy link
Contributor

@DanG100 DanG100 commented May 11, 2023

  • Updated code gen logic: generate calls into the generic create/remove/get/set funcs in Translator class
  • Regenerate code in dataplane/standalone/sai/**
  • Create generic create/remove/get/set method in Translator class that for all all sai object types (unimplemented for now)
  • replace custom logger with glog
  • I will rewrite switch.cc and implement the translator methods in a followup PR

@github-actions
Copy link

github-actions bot commented May 11, 2023

Pull Request Test Coverage Report for Build 4954199114

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 9 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.0008%) to 2.258%

Files with Coverage Reduction New Missed Lines %
sysrib/zapi.go 9 76.8%
Totals Coverage Status
Change from base Build 4952946874: -0.0008%
Covered Lines: 11914
Relevant Lines: 527705

💛 - Coveralls

Copy link
Contributor

@wenovus wenovus left a comment

Choose a reason for hiding this comment

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

Thanks, can you give a brief overview of the parts of the change that's harder to understand so it's quicker for me to review? Without context I really don't know what is the logic behind the changes and I often find myself reading everything just to understand what it's doing first, which takes much longer than if there were context available beforehand so I know what I'm looking for while reviewing.

@DanG100 DanG100 requested a review from wenovus May 11, 2023 23:07
@DanG100
Copy link
Contributor Author

DanG100 commented May 11, 2023

Thanks, can you give a brief overview of the parts of the change that's harder to understand so it's quicker for me to review? Without context I really don't know what is the logic behind the changes and I often find myself reading everything just to understand what it's doing first, which takes much longer than if there were context available beforehand so I know what I'm looking for while reviewing.

updated desc

@DanG100 DanG100 merged commit a4d7f40 into main May 12, 2023
@DanG100 DanG100 deleted the rif branch May 12, 2023 16:11
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