-
Notifications
You must be signed in to change notification settings - Fork 1
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
Privacy and security - Part I #5
base: main
Are you sure you want to change the base?
Conversation
… read names from a csv file
a6476f6
to
4d09a74
Compare
4d09a74
to
4008672
Compare
meetup_ballot/ballot.py
Outdated
logging.info( | ||
"Bad Member name: {} (ID: {})".format(member_name, member_id) | ||
) | ||
if member_id in read_name_exceptions(name_exceptions_csv): |
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.
I think you want to move the call to read_name_exceptions
outside the loop, otherwise the file will be re-read on every iteration.
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.
Good point! Wasn't sure how to run the code end-to-end, did run the tests and they were passing but I think there is some bug somewhere in how unittest is set up in virtualenv, can we rely on Travis ?
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.
Pushed changes, but I need to see how to run the tests properly!
meetup_ballot/ballot.py
Outdated
def read_name_exceptions(name_exceptions_csv): | ||
""" Read member names from csv files""" | ||
member_ids = [] | ||
with open(name_exceptions_csv, newline='') as name_exceptions: |
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.
Is there a specific reason to disable translation of newlines?
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.
Not really, I will remove it.
62b45ee
to
32779cd
Compare
32779cd
to
d270c95
Compare
name_exceptions_csv
filedoes_member_name_looks_like_spam
function