-
Notifications
You must be signed in to change notification settings - Fork 4
Conversation
else: | ||
return ObjectDoesNotExist | ||
result = False |
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.
@vubo I don't think that it is necessary to have an else clause since by the time one hits the return statement the result will be unchanged and false or delete occurred and True.
@willingc thank you for the comments, I corrected this here: vubo/vms@62c95a7 |
from organization.models import Organization | ||
from volunteer.models import Volunteer | ||
from volunteer.services import * | ||
from volunteer.services import delete_volunteer, delete_volunteer_resume, get_all_volunteers, get_volunteer_by_id, get_volunteer_resume_file_url, get_volunteers_ordered_by_first_name, has_resume_file, search_volunteers |
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.
Nice! If you wish to fix the line length here. You could put in parens (delete_volunteer, .... search_volunteers), then you can break the line
Please rebase and squash the commit history. I'm happy to merge once that is done. 👍 |
@vubo If you run into any roadblocks with the rebase and squash, the PyLadies issue tracker has some comments that I left for others. Also, after you squash, you can push the newly squashed commits to this PR. You do not need to close this PR and create a new PR. Simply push to your same branch (you may need to force push) and the PR should auto-magically update. |
Unit tests for volunteer Refactor unit tests for volunteer Fix import
@willingc thank you, I didn't get it completely correctly this time, but I hope, I will commit it next time 😁 |
@vubo No worries. You are doing a great job! |
#29