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

💀 Deathdate … and anniversary! 🎉 #472

Merged
merged 3 commits into from Aug 2, 2016
Merged

💀 Deathdate … and anniversary! 🎉 #472

merged 3 commits into from Aug 2, 2016

Conversation

DeepDiver1975
Copy link
Member

No description provided.

@DeepDiver1975 DeepDiver1975 added this to the 1.4 milestone Jul 29, 2016
@mention-bot
Copy link

@DeepDiver1975, thanks for your PR! By analyzing the annotation information on this pull request, we identified @jancborchardt, @Henni and @skjnldsv to be potential reviewers

@jancborchardt
Copy link
Member

jancborchardt commented Jul 29, 2016

Not sure if I should be worried about you or me now …

@codecov-io
Copy link

codecov-io commented Jul 29, 2016

Current coverage is 12.13% (diff: 100%)

Merging #472 into master will not change coverage

@@             master       #472   diff @@
==========================================
  Files            45         45          
  Lines           816        816          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits             99         99          
  Misses          717        717          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 02e4b64...18859ba

template: 'date'
},
deathdate: {
readableName: t('contacts', 'Date of Death'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct capitalization should be »Date of death«, only first character capitalized.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix right away .... thx

@DeepDiver1975
Copy link
Member Author

Not sure if I should be worried about you or me now …

no worry this introduces anniversary as well ....

@jancborchardt jancborchardt changed the title Deathdate 💀 Deathdate … and anniversary! 🎉 Jul 29, 2016
@DeepDiver1975
Copy link
Member Author

sync to calendar is in core - owncloud/core#25655

@jancborchardt
Copy link
Member

So @ChristophWurst noted something: Does it really make sense to save date of death for a person in Contacts? Cause … you will not contact them anymore really.

I guess it makes sense to remember someone though, or just for completeness … I just wanna make sure we’re not just stupidly implementing the spec, because well, this in particular is a very slim usecase. ;)

@spoorun
Copy link

spoorun commented Jul 29, 2016

@jancborchardt A good point. We had thought of that, and from a personal perspective, I retain contacts who have died, often for a memory or commemoration - for many people it is an important a date to remember someone - as important as a birthday is of someone still alive. So it's not a huge use case, but certainly not a 'very slim' one. As people grow older, alas that usecase also grows.

@jancborchardt
Copy link
Member

Ok, just wanted to make sure it’s a conscious decision adding this, not just cause you’re in the process of adding anniversary anyway. :)

@MariusBluem
Copy link
Contributor

MariusBluem commented Jul 29, 2016

Our satanistic users will like that 😈

@spoorun
Copy link

spoorun commented Jul 29, 2016

I guess there's nothing preventing future events being recorded either.. for those who are exceptionally well-planned!

@DeepDiver1975 DeepDiver1975 merged commit b8c3e06 into master Aug 2, 2016
@DeepDiver1975 DeepDiver1975 deleted the deathdate branch August 2, 2016 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants