gNOI Cold Reboot - Integrated tests #20711
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
3 times, most recently
from
d4ba3f9 to
ad91226
Compare
|
@hdwhdw @vvolam @qiluo-msft Pls review |
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
from
ad91226 to
ee29b8b
Compare
hdwhdw
suggested changes
Nov 25, 2024
Comment on lines
23
to
28
| #define ONE_SECOND (1) | ||
| #define TWO_SECONDS (2) | ||
| #define TENTH_SECOND_MS (100) | ||
| #define HALF_SECOND_MS (500) | ||
| #define ONE_SECOND_MS (1000) | ||
| #define FIFTEEN_HUNDRED_MS (1500) | ||
| #define TWO_SECONDS_MS (2000) |
There was a problem hiding this comment.
This is a level of indirection that make it:
- Hard to change going forward.
- Does not enforce units (miliseconds, seconds), i.e. there is nothing that prevents you to write a
ONE_SECONDin a parameter which expect miliseconds.
It is of high risk of introducing bugs for unsuspecting maintainers. Please remove it.
For Google code, consider using https://abseil.io/docs/cpp/guides/time
There was a problem hiding this comment.
There is a challenge in using absl. So, retaining this code
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
4 times, most recently
from
8c4cb6c to
efa4a2b
Compare
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
5 times, most recently
from
663ac61 to
8d52b9b
Compare
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
from
8d52b9b to
446d015
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
from
446d015 to
00176e7
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
from
00176e7 to
2253140
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
from
2253140 to
e9bfc38
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
from
e9bfc38 to
5bec465
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rkavitha-hcl
force-pushed
the
gnoi_cr_PR3
branch
from
5bec465 to
de106e0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why I did it
Work item tracking
How I did it
How to verify it
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Description for the changelog
Added UT cases for Cold Reboot feature
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)