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

Clock commands management HLD #1219

Merged
merged 22 commits into from
Apr 28, 2023

Conversation

Meir-renford
Copy link
Contributor

@Meir-renford Meir-renford commented Jan 16, 2023

This is a PR for community review of clock commands management high level design.

Repo PR title State
sonic-utilities [Clock] Implement clock CLI GitHub issue/pull request detail
sonic-buildimage [Clock] Implement clock CLI GitHub issue/pull request detail
sonic-host-services [Clock] Implement clock CLI GitHub issue/pull request detail
sonic-mgmt [Clock] Implement clock CLI GitHub issue/pull request detail

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 16, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

doc/Clock commands/clock_managment_hld.md Outdated Show resolved Hide resolved
doc/Clock commands/clock_managment_hld.md Outdated Show resolved Hide resolved
doc/Clock commands/clock_managment_hld.md Outdated Show resolved Hide resolved
@zhangyanzhao
Copy link
Collaborator

The HLD is reviewed in SONiC community meeting on 1/17/2023

@zhangyanzhao
Copy link
Collaborator

Please remember to update https://github.com/sonic-net/sonic-utilities/blob/master/doc/Command-Reference.md with the new CLIs

doc/Clock commands/clock_managment_hld.md Outdated Show resolved Hide resolved
doc/Clock commands/clock_managment_hld.md Outdated Show resolved Hide resolved
doc/Clock commands/clock_managment_hld.md Show resolved Hide resolved
doc/Clock commands/clock_managment_hld.md Show resolved Hide resolved
doc/Clock commands/clock_managment_hld.md Outdated Show resolved Hide resolved
Update HLD to community with all comments recieved.
added additional tests
@liat-grozovik
Copy link
Collaborator

@venkatmahalingam, @madhupalu @bsun-sudo @dharmaraj-gurusamy HLD is updated based on comments provided, please review and let us know if you have any further concern. if not please approve.

Copy link
Collaborator

@dharmaraj-gurusamy dharmaraj-gurusamy left a comment

Choose a reason for hiding this comment

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

Approved the HLD with the suggestion to fine tune the logic for upgrade scenario.

changed the examples according to exact implementation.
@Yuval-Mellanox
Copy link
Contributor

@yxieca @madhupalu when do you expect to complete the code review? we want to merge it before the branching out happens

@fastiuk fastiuk self-assigned this Apr 18, 2023
@fastiuk fastiuk requested review from fastiuk and removed request for fastiuk April 18, 2023 13:32
dgsudharsan
dgsudharsan previously approved these changes Apr 18, 2023
madhupalu
madhupalu previously approved these changes Apr 18, 2023
Copy link
Contributor

@madhupalu madhupalu left a comment

Choose a reason for hiding this comment

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

thanks for addressing the comments

bsun-sudo
bsun-sudo previously approved these changes Apr 19, 2023
@liat-grozovik liat-grozovik merged commit a094d2a into sonic-net:master Apr 28, 2023
@liat-grozovik
Copy link
Collaborator

PR is now merged. as the feature is planed for 202305 appreciate code PR review and feedback to be able not to skip the release timeline.

@liat-grozovik
Copy link
Collaborator

@Meir-renford @fastiuk the code PRs description is not aligned with the PR subjects. Can you please align?
for PRs which has nothing to do with the CLI please change the PR subject to align with the motivation of the PR.

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.