Skip to content

Conversation

yoichi
Copy link
Contributor

@yoichi yoichi commented Oct 8, 2020

before
スクリーンショット 2020-10-08 19 07 38

after
スクリーンショット 2020-10-08 19 08 36

Copy link
Owner

@o2sh o2sh left a comment

Choose a reason for hiding this comment

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

Using the email as a UID is definetely the right way to do it. Thanks @yoichi

I suggested some renaming to make it clearer.

src/info.rs Outdated
let mut names = std::collections::HashMap::new();
let mut total_commits = 0;
for line in git_history {
let commit_author = line.split('\t').collect::<Vec<_>>()[1].to_string();
Copy link
Owner

Choose a reason for hiding this comment

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

This should probably be called commit_author_email

let mut total_commits = 0;
for line in git_history {
let commit_author = line.split('\t').collect::<Vec<_>>()[1].to_string();
let author_name = line.split('\t').collect::<Vec<_>>()[2].to_string();
Copy link
Owner

Choose a reason for hiding this comment

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

This should probably be called commit_author_name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I think the following variable names are good because of the relationship with the format string of git log.

  • %an = author_name
  • %ae = author_email

I've renamed commit_author to author_email in c68e021

src/info.rs Outdated

fn get_authors(git_history: &[String], n: usize) -> Vec<(String, usize, usize)> {
let mut authors = std::collections::HashMap::new();
let mut names = std::collections::HashMap::new();
Copy link
Owner

Choose a reason for hiding this comment

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

This should probably be called author_name_by_email

@o2sh o2sh merged commit ca58871 into o2sh:master Oct 8, 2020
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.

2 participants