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

Refactored Event to store a person instance attribute #25

Merged
merged 1 commit into from Oct 13, 2021

Conversation

romanZupancic
Copy link
Contributor

@romanZupancic romanZupancic commented Oct 13, 2021

Actually makes these series of classes usable, but most of the event implementation still sucks. I will work on adding #16 in a future pull request, along with any tagging features we want implemented for events.

For this pull request: Do we want to get rid of the event heirarchy altogether, and just use tags instead on a single concrete event class?

@nathan-hansen
Copy link
Contributor

We can't entirely remove the hierarchy, since anniversaries function differently than birthdays (reminding at 1, 3, and 6 months and then each following year compared to just every year). for now i think we put anniversaries on the backburner though.

@nathan-hansen nathan-hansen merged commit c94a8d7 into main Oct 13, 2021
@nathan-hansen nathan-hansen deleted the event_refactor branch October 13, 2021 01:23
@romanZupancic
Copy link
Contributor Author

We can't entirely remove the hierarchy, since anniversaries function differently than birthdays (reminding at 1, 3, and 6 months and then each following year compared to just every year). for now I think we put anniversaries on the backburner though.

That is a very good point. I'm tagging #16 for when we implement recurring reminders.

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.

None yet

2 participants