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

Remove Author.email field and merge authors by name #10

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

filodej
Copy link

@filodej filodej commented Mar 29, 2013

For example John Smith <john.smith@example.com> and John Smith <jsmith@example.com> are considered as a single author John Smith regardless of the different email addresses.

As the URL for author details is constructed from Autor.dirname (as opposed to Author.email - see dc4d2a0) the original version does not work properly.

This change is just a workaround and is not intended for a direct merge, I created it just to trigger a discussion.

However, for me personally it feels as a suitable quickfix until the whole issue is properly addressed.

For example "John Smith <john.smith@example.com>" and "John Smith <jsmith@example.com>"
are considered as a single author regardless of the different email addresses.

This change is just a workaround.

But as the URL for author details is constructed from Autor.dirname 
(as opposed to Author.email - see dc4d2a0) the original version does not work anyway.

This seems to me as a suitable quickfix until the whole issue is properly addressed.
@zzjin
Copy link

zzjin commented Mar 30, 2013

+1

@tomgi
Copy link
Owner

tomgi commented Apr 1, 2013

Thanks for pointing the problem with author details URL.

I think better solution than removing email field from author would be to:

  • save collection of author's emails in author class
  • add option in API and CLI to merge authors with specified emails (so they will be considered as a single author)

@filodej
Copy link
Author

filodej commented Apr 1, 2013

Hello Tomasz,
Yes, the approach you described is definitely better.
Ideally there could be also an option to merge all authors (to avoid listing the e-mails and still get the behavior I achieved with my hack).
BTW. thank you very much for a great piece of software! The presentation looks so much nicer that with the original gitstats.
Cheers, Filodej.

@tomgi tomgi mentioned this pull request Aug 29, 2013
@miketheman
Copy link

Ping for this - CLI-wise, I agree that a toggle to allow the operator to turn off the email behavior is a good idea.
Where did this idea stall?

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

4 participants