-
Notifications
You must be signed in to change notification settings - Fork 40
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
Outlook bug #710
Outlook bug #710
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.
- Added comments.
- Please add tests for this functionality with
8000
.
viadot/sources/outlook.py
Outdated
if sum(list(map(len, [recivers, add_string]))) >= 8000: | ||
break | ||
else: | ||
recivers += add_string |
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.
My suggestion - what do you think about using only 1 if to this case?
if sum(list(map(len, [recivers, add_string]))) >= 8000: | |
break | |
else: | |
recivers += add_string | |
if sum(list(map(len, [recivers, add_string]))) < 8000: | |
recivers += add_string | |
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.
The end is the same, because, I need to break the for
loop when it reaches 8000, after that limit you are iterating without any purpose.
If I use your sentence, then I would need an else: break
which is basically the same as it is already there.
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.
Please add or modify tests for new features:
- break if
>= 8000
- handling
receivers
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.
created a new parameter and updated test file.
Please reduce logs to only directories where we can find some messages. In the previous version, a user was able to see everything. (It was a long log message) |
I have updated the source file to print out only folders with some messages. |
Summary
subject
parameter occasionally appears an extra separator, which has been solved by replacing it with a space.receivers
parameter is longer than 8000 (maximum length for creating avarchar
) the string is truncated to the latest email added to thisreceivers
parameter.Importance
It happened in the past but I would be necessary to avoid similar situations in the future.
Checklist
This PR:
CONTRIBUTING.md
CHANGELOG.md