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

Update hp_comware_display_interface_brief.textfsm #1964

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

Update hp_comware_display_interface_brief.textfsm #1964

wants to merge 5 commits into from

Conversation

juliencorsini
Copy link

Fixed regexp of hp_comware_display_interface_brief.textfsm for duplex value.
Fixes #1949

@juliencorsini
Update hp_comware_display_interface_brief.textfsm
920d421 
Fixed regexp of hp_comware_display_interface_brief.textfsm for duplex value.
Fixes networktocode#1949
@juliencorsini
Copy link
Author

I may have to check if #1955 does not fix this issue.
I just saw this after creating the PR.

jmcgill298
jmcgill298 requested changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@jmcgill298 jmcgill298 left a comment

Choose a reason for hiding this comment

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

Thanks @juliencorsini, can you please update the test case to provide an example of what this is fixing? I also suspect that we will need to account for the ( and )

juliencorsini and others added 2 commits January 7, 2025 12:41
@juliencorsini @mjbear
Update hp_comware_display_interface_brief.textfsm
44cff27 
Fixed regexp of hp_comware_display_interface_brief.textfsm for duplex value.
Fixes networktocode#1949
@mjbear
Add test data for HP Comware full auto ports
9ca634f 
(That's ports set to auto-negotiate, but that had negotiated Full
duplex)

* Test data from issue#1949
@mjbear
Copy link
Contributor

mjbear commented Jan 7, 2025

I may have to check if #1955 does not fix this issue. I just saw this after creating the PR.

Julien, your PR is for "brief" and that one is the normal non-brief output.

All good though.
I'll have a PR over to you to show you about the test data piece that Jacob referred to. From there you can mimic that requirement for any other PRs you submit.

@mjbear mjbear mentioned this pull request Jan 7, 2025
Merged
@mjbear
Copy link
Contributor

mjbear commented Jan 7, 2025

@juliencorsini
I created juliencorsini#1 for your review.
After your review, you will be able to "merge" the changes into your patch-2 branch so they show up on this PR.

Any questions, please ask.

@mjbear mjbear mentioned this pull request Jan 7, 2025
Open
@mjbear
Copy link
Contributor

mjbear commented Jan 14, 2025 •
edited
Loading

@juliencorsini I created juliencorsini#1 for your review. After your review, you will be able to "merge" the changes into your patch-2 branch so they show up on this PR.

Any questions, please ask.

@juliencorsini
Did you see this message for display interface brief?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmcgill298 jmcgill298 jmcgill298 requested changes

@itdependsnetworks itdependsnetworks Awaiting requested review from itdependsnetworks itdependsnetworks is a code owner

@jvanderaa jvanderaa Awaiting requested review from jvanderaa jvanderaa is a code owner

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.

hp_comware_display_interface_brief.textfsm failing on Duplex value
3 participants
@juliencorsini @mjbear @jmcgill298