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

Add max error length #473

Merged
merged 3 commits into from
Oct 29, 2022
Merged

Add max error length #473

merged 3 commits into from
Oct 29, 2022

Conversation

jacadzaca
Copy link
Collaborator

fix #472

return comps[0]

def _format_error(error_description, bad_input, elipsis='[...]'):
# there's three character more in the error, ie. ' ' x2 and a ':'
Copy link
Member

Choose a reason for hiding this comment

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

I think, a docstring would be nice, also linking to tje issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think a docstring would be redundant and the issue's linked in the commit message.

@niccokunzmann niccokunzmann merged commit 12f34ae into master Oct 29, 2022
@niccokunzmann
Copy link
Member

Thanks! New version?

@jacadzaca
Copy link
Collaborator Author

Maybe let's merge #479 into master first?

@niccokunzmann
Copy link
Member

That might take a bit of time. Whenever you or I or anyone else feel like it, a new version can be generated.

def test_error_message_doesnt_get_too_big(calendars, calendar_name):
with pytest.raises(ValueError) as exception:
calendars[calendar_name]
assert len(str(exception).split(': ')[1]) <= 100
Copy link
Collaborator

Choose a reason for hiding this comment

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

That should probably be .split(': ', 1) so a second ': ' does not hide later parts of the message from the assertion.

@niccokunzmann niccokunzmann deleted the add_max_error_length branch November 2, 2022 00:30
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.

[BUG] Calendar.from_ical() issue
3 participants