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

Fixes #55: Check commit comment length #56

Merged
merged 3 commits into from
Apr 16, 2018

Conversation

Ichabond
Copy link
Contributor

Raise an InvalidInputError when the commit message is longer than 60 characters, as this breaks IOS-XR.

Raise an InvalidInputError when the commit message is longer than 60 characters, as this breaks IOS-XR.
@coveralls
Copy link

coveralls commented Nov 27, 2017

Coverage Status

Coverage decreased (-0.3%) to 87.456% when pulling 6317678 on Ichabond:master into b83f51e on fooelisa:master.

pyIOSXR/iosxr.py Outdated
if len(comment) <= 60:
rpc_command += ' Comment="%s"' % comment
else:
raise InvalidInputError('comment needs to be shorter than 60 characters', self)

Choose a reason for hiding this comment

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

It can be up to and including 60 characters

Rephrase the commit comment error to better reflect the limits.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 87.456% when pulling 9ab98f0 on Ichabond:master into b83f51e on fooelisa:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 87.456% when pulling 9ab98f0 on Ichabond:master into b83f51e on fooelisa:master.

@coveralls
Copy link

coveralls commented Nov 27, 2017

Coverage Status

Coverage remained the same at 87.719% when pulling 2661791 on Ichabond:master into b83f51e on fooelisa:master.

@Ichabond
Copy link
Contributor Author

Thanks to @loverend for the reworded message.

pyIOSXR/iosxr.py Outdated
if len(comment) <= 60:
rpc_command += ' Comment="%s"' % comment
else:
raise InvalidInputError('comment should be no more than 60 characters', self)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should just trim the commit message when it's longer than 60 characters.

Instead of raising an exception, just truncate the comment length to the maximum of 60
@mirceaulinic mirceaulinic merged commit a60b550 into fooelisa:master Apr 16, 2018
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