-
Notifications
You must be signed in to change notification settings - Fork 399
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
updated P2 pattern to make sn optional #601
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contributing! Left a comment
output = self.device.execute(self.cli_command) | ||
|
||
return super().cli(output=output) | ||
''' show_inventory.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's something a little odd happening here.
Only p2 was changed but git is highlighting around 100 lines as being changed. Perhaps did you commit files that had different line endings?
Please investigate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats odd, let me go check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Lukas,
It is the line ending, but pycharm doesn't give me the option to change to CRLF (it's grey out). Not sure if this is a MAC thing. I tried changing the file encoding too. The most frustrating thing, is the pycharm diff only shows that single line as being changed. I only see the problem after I push to github and run diff there. Not sure how to sort this any ideas?
Nice! I think that fixed the line endings. I also went ahead and removed trailing white space from the lines. Please add a changelog (instructions are in this repos README) |
name_dict['sn'] = group['sn'] | ||
continue | ||
|
||
name_dict['sn'] = group['sn'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If sn
is optional, then a check will be needed here
name_dict['sn'] = group['sn'] | |
if group['sn']: | |
name_dict['sn'] = group['sn'] |
Also, it seems one of the unit tests is failing
You can test your changes with the following command
from the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add change log for your changes.
@aclaughan Are you still working on this PR? |
Description
Changed P2 regex to make
sn
field optionalMotivation and Context
show_inventory.py was only parsing lines that included a serial number