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

Remove Write to system log API from Vireo #694

Merged
merged 2 commits into from
Sep 28, 2021

Conversation

MalleshSushmitha
Copy link
Contributor

@MalleshSushmitha MalleshSushmitha commented Sep 20, 2021

The Write to system log API for the Vireo target is now built using JSLI in the NXG code base(it was previously using an SLI).
So we no longer need this function definition in the Vireo code base.

Note:
I have also removed the tests related to the same function.
PR for changes on the NXG codebase: PR

@MalleshSushmitha MalleshSushmitha marked this pull request as ready for review September 20, 2021 13:37
@MalleshSushmitha MalleshSushmitha marked this pull request as draft September 20, 2021 13:38
@rajsite
Copy link
Member

rajsite commented Sep 20, 2021

@MalleshSushmitha is there a reason this was changed to a Draft PR? Is it not ready for review?

@MalleshSushmitha
Copy link
Contributor Author

MalleshSushmitha commented Sep 20, 2021

@MalleshSushmitha is there a reason this was changed to a Draft PR? Is it not ready for review?

@rajsite I was not aware that owners will be added as soon as I create a PR here. I changed it to draft since I haven't gotten this reviewed locally by anyone in the team.

@rajsite
Copy link
Member

rajsite commented Sep 20, 2021

@MalleshSushmitha Thanks for clarifying. Just making sure it was intentional. Will review when it is changed out of draft 👍

@MalleshSushmitha MalleshSushmitha marked this pull request as ready for review September 21, 2021 11:49
@MalleshSushmitha MalleshSushmitha changed the title Remove the code for Write to system log API from Vireo Remove Write to system log API from Vireo Sep 21, 2021
@MalleshSushmitha
Copy link
Contributor Author

@MalleshSushmitha Thanks for clarifying. Just making sure it was intentional. Will review when it is changed out of draft 👍

@rajsite This is ready for review now.

@rajsite
Copy link
Member

rajsite commented Sep 27, 2021

@MalleshSushmitha sorry for the delay, I expect to review this tomorrow

@sanmut
Copy link
Contributor

sanmut commented Sep 28, 2021

@MalleshSushmitha , would you mind referencing the NXG side commits/PRs that implement the feature for reference?

@MalleshSushmitha
Copy link
Contributor Author

@sanmut I have modified to include a link to the PR in the description.

@rajsite
Copy link
Member

rajsite commented Sep 28, 2021

Bypassing @cglzaguilar and @spathiwa who are required reviewers but this is a pretty simple change that effectively reverts #570

@rajsite rajsite merged commit 0655706 into ni:main Sep 28, 2021
@rajsite
Copy link
Member

rajsite commented Sep 28, 2021

@MalleshSushmitha A new Vireo was released with these changes. Make sure to update Vireo in ASW.

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.

4 participants