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.

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

Add hostif trap for NTP #1453

Merged
merged 2 commits into from
Apr 5, 2022
Merged

Add hostif trap for NTP #1453

merged 2 commits into from
Apr 5, 2022

Conversation

vivekmoorthy
Copy link
Contributor

@mikeberesford
Copy link
Contributor

@kcudnik @rck-innovium @JaiOCP @kperumalbfn Mind reviewing this PR? It's similar to #1436

vivekmoorthy
vivekmoorthy commented Mar 23, 2022
View reviewed changes
inc/saihostif.h Outdated Show resolved Hide resolved
JaiOCP
JaiOCP approved these changes Mar 23, 2022
View reviewed changes
Copy link
Contributor

@JaiOCP JaiOCP left a comment

Choose a reason for hiding this comment

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

Looks good to me

@vivekmoorthy
Copy link
Contributor Author

@rck-innovium Could you help with this review?

@vivekmoorthy
Copy link
Contributor Author

@kcudnik I see the PR is waiting on "LGTM analysis: Python"
Does this need to be triggered again? Suggestions?

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 30, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 30, 2022

not sure why this hanged on python, no changes are here made

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 30, 2022

if this retrigger will not help, please add force empty commit to this branch this will retrigger all pipelines

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 30, 2022

i'm not able to retrigger python from github page @vivekmoorthy, please add empty commit here to retrigger

retrigger python LGTM
ab8033c 
Signed-off-by: Vivek Ramamoorthy <[email protected]>
@vivekmoorthy
Copy link
Contributor Author

Thanks. retriggered and now the checks have passed.

@vivekmoorthy
Copy link
Contributor Author

@rck-innovium @JaiOCP @kperumalbfn could you provide your approvals again due to retrigger.

@vivekmoorthy
Copy link
Contributor Author

@kcudnik If approvals look ok, would you be able to help with merging this PR?

@kcudnik
Copy link
Collaborator

kcudnik commented Apr 4, 2022

was this discussed and approved on latest SAI community meeting?

@vivekmoorthy
Copy link
Contributor Author

was this discussed and approved on latest SAI community meeting?

This PR was discussed 2 weeks back in SAI meeting and the decision was to split the Traps to NTPCLIENT and NTPSERVER.

@lguohan for visibility.

@rlhui rlhui merged commit fc7e8a5 into opencomputeproject:master Apr 5, 2022
@rlhui rlhui mentioned this pull request Mar 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kcudnik kcudnik kcudnik approved these changes

@kperumalbfn kperumalbfn kperumalbfn approved these changes

@JaiOCP JaiOCP JaiOCP approved these changes

@rck-innovium rck-innovium rck-innovium 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.
7 participants
@vivekmoorthy @mikeberesford @kcudnik @kperumalbfn @JaiOCP @rck-innovium @rlhui