-
Couldn't load subscription status.
- Fork 1
Privacy and security - Part I #5
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
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_csvfiledoes_member_name_looks_like_spamfunction