Add new template for Fsas Si-R show ether
#1971
Conversation
caribouHY
requested review from
itdependsnetworks,
jmcgill298 and
jvanderaa
as code owners
| status: "100M Full MDI" | ||
| status_auto: "" | ||
| status_duplex: "Full" | ||
| status_mdi: "MDI" | ||
| status_speed: "100M" |
There was a problem hiding this comment.
It's a bit of a bummer the status data is captured twice.
I do notice that's likely because of the offline (backup) and disable statuses.
There was a problem hiding this comment.
Thank you for the review. Would you prefer not to capture STATUS when not offline or disalble?
| status: "100M Full MDI" | |
| status_auto: "" | |
| status_duplex: "Full" | |
| status_mdi: "MDI" | |
| status_speed: "100M" | |
| status: "" | |
| status_auto: "" | |
| status_duplex: "Full" | |
| status_mdi: "MDI" | |
| status_speed: "100M" |
^status\s+:\s+(${STATUS_AUTO}\s+)?${STATUS_SPEED}\s+${STATUS_DUPLEX}\s+${STATUS_MDI}\s*$$
There was a problem hiding this comment.
Thank you for the review. Would you prefer not to capture
STATUSwhen notofflineordisalble?^status\s+:\s+(${STATUS_AUTO}\s+)?${STATUS_SPEED}\s+${STATUS_DUPLEX}\s+${STATUS_MDI}\s*$$
@jmcgill298 @jvanderaa @itdependsnetworks
What are your thoughts on the STATUS sometimes capturing data that is captured in other capture groups?
The downside here is that the STATUS capture group would have to become much more rigid (to be the two statuses in the present test data) so it doesn't match the speed/duplex/MDI.
Update: Change sentence grammar.
There was a problem hiding this comment.
I think if the goal is to split the status field into multiple capture groups, then the status field should capture auto, disable, and offline text, and then it would be an empty string if it is set to a particular speed.
There was a problem hiding this comment.
I think if the goal is to split the status field into multiple capture groups, then the status field should capture
auto,disable, andofflinetext, and then it would be an empty string if it is set to a particular speed.
I like this idea.
I cooked up some suggestions.
Co-authored-by: Michael Bear <[email protected]>
| Value STATUS (.+?) | ||
| Value STATUS_AUTO (auto) |
There was a problem hiding this comment.
Support one and two word statuses (ex: offline (backup)).
| Value STATUS (.+?) | |
| Value STATUS_AUTO (auto) | |
| Value STATUS (\S+(\s+\S+)?) |
There was a problem hiding this comment.
The "commit suggestion" button can help easily merge in suggestions. Thank you.
There was a problem hiding this comment.
STATUS have to support the following status:
offlineoffline (backup)offline (startup down)offline (recovery manual)
I added a test case.
Co-authored-by: Michael Bear <[email protected]>
Co-authored-by: Michael Bear <[email protected]>
Co-authored-by: Michael Bear <[email protected]>
Co-authored-by: Michael Bear <[email protected]>
I added a new template for Fsas Si-R
show ether.