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

Feature/show logging level #729

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Feature/show logging level #729

wants to merge 17 commits into from

Conversation

rohit04saluja
Copy link
Contributor

@rohit04saluja rohit04saluja commented Feb 14, 2023 •
edited
Loading

Description

Added classes and parsing for show logging level {facility}.

Motivation and Context

Need parser for show logging level {facility} as it's not present

Impact (If any)

None

Screenshots:

Checklist:

  • I have updated the changelog.
  • I have updated the documentation (If applicable).
  • I have added tests to cover my changes (If applicable).
  • All new and existing tests passed.
  • All new code passed compilation.

@rohit04saluja rohit04saluja requested a review from a team as a code owner February 14, 2023 16:30
GerriorL
GerriorL reviewed Feb 14, 2023
View reviewed changes
Copy link
Contributor

@GerriorL GerriorL left a comment

Choose a reason for hiding this comment

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

Hi, thanks for your contribution! It looks good, but there's a couple of changes that should be made

src/genie/libs/parser/nxos/show_logging.py Outdated Show resolved Hide resolved
src/genie/libs/parser/nxos/show_logging.py Outdated Show resolved Hide resolved
@GerriorL GerriorL self-requested a review February 14, 2023 21:41
GerriorL
GerriorL previously requested changes Feb 14, 2023
View reviewed changes
Copy link
Contributor

@GerriorL GerriorL left a comment

Choose a reason for hiding this comment

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

I left a couple of comments, please try and address them if possible

omehrabi
omehrabi requested changes Feb 24, 2023
View reviewed changes
Copy link
Contributor

@omehrabi omehrabi left a comment

Choose a reason for hiding this comment

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

Hi
thank you for contributing left comment

@Taarini
Copy link
Contributor

Taarini commented Mar 10, 2023

Hi

Can you please address the review comments

Rohit Saluja and others added 5 commits March 21, 2023 14:21
@rohit04saluja
Copy link
Contributor Author

Hi @Taarini @omehrabi can you please highlight the pending comments ?

If there is nothing can we please merge this PR ?

@rohit04saluja
Copy link
Contributor Author

@Taarini can you please highlight the pending comments?

@Taarini Taarini requested review from omehrabi and Taarini and removed request for GerriorL January 30, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GerriorL GerriorL GerriorL left review comments

@omehrabi omehrabi Awaiting requested review from omehrabi

@Taarini Taarini Awaiting requested review from Taarini

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rohit04saluja @Taarini @GerriorL @omehrabi