feat: commit facts #58

astansler · 2017-11-06T16:50:22Z

Add new facts calculation:

Average number of lines per commit (used for fun fact)
Number of commits in repo (used for calculation of prev fun fact between all user's repos)
Average length of line (used for fun fact)
Number of locs for through all commits (used for calculation of prev fun fact between all user's repos)
Number of commits per number of lines (used for histogram in did you know section)

asurkov · 2017-11-06T21:36:05Z

src/main/kotlin/app/hashers/CommitCrawler.kt

+            }
+            new.numLinesDeleted = new.diffs.fold(0) { total, file ->
+                total + file.getAllDeleted().size
+            }


why do you move this code? I find it reasonable to keep this code in CommitHasher, which is all about a commit analysis/processing.

Because it's general commit stats that could be used in multiple places, e.g. in facthasher.

I see the point

asurkov · 2017-11-06T21:40:27Z

src/main/kotlin/app/hashers/FactHasher.kt

+        }
+
+        // RepoTeamSize.
+        fsRepoTeamSize.add(email)


it is a funny to have teamSize as team email storage, fsRepoTeam would more reasonable name

Actually, I'll make it simpler to calculate.

asurkov · 2017-11-06T21:47:31Z

src/main/kotlin/app/hashers/FactHasher.kt

+
+        // Commits.
+        val numCommits = fsCommitNum[email]!! + 1
+        val numLinesCurrent = commit.numLinesAdded - commit.numLinesDeleted


I don't understand this metric: if a user removes lines only, then it's average is negative? If the user always adds same amount of lines that he removes, then the average is 0?

Anyway at least it'd be good to comment a metric when it is defined.

This should be sum of added and deleted.

asurkov · 2017-11-06T22:00:21Z

src/main/kotlin/app/hashers/FactHasher.kt

+        fsLineNum[email] = fsLineNum[email]!! + addedLines.size
+
+        fsLinesPerCommits[email]!![numCommits - 1] =
+            fsLinesPerCommits[email]!![numCommits - 1] + addedLines.size


you don't need make !! twice, right?

Smart cast didn't work for this case. += worked and it's not a problem anymore.

asurkov · 2017-11-06T22:08:57Z

.gitignore

 /ultimate/ideaSDK
 /ultimate/out
 /ultimate/tmp
-src/main/resources/data/models/


the change seems unrelated, why it is?

Models got deleted from resources, but we forgot to delete them from gitignore.

asurkov · 2017-11-06T22:30:34Z

src/main/kotlin/app/hashers/FactHasher.kt

+        }
+        fsLineNum[email] = fsLineNum[email]!! + addedLines.size
+
+        fsLinesPerCommits[email]!![numCommits - 1] =


asurkov · 2017-11-06T22:59:21Z

src/main/kotlin/app/hashers/FactHasher.kt

+            fsCommitNum.put(author, 0)
+            fsLineLenAvg.put(author, 0.0)
+            fsLineNum.put(author, 0)
+            fsLinesPerCommits.put(author, Array(rehashes.size) {0})


that may be as large as half million items array, for example in case of gecko-dev repo. Are you sure about keeping all of this in the memory? Can you do the bin computations as addCommitsPerLinesFacts has on the go?

Good idea, I've add todo to future implementation.

asurkov · 2017-11-06T23:02:32Z

src/main/kotlin/app/hashers/FactHasher.kt

        }
    }
+
+    private fun calcIncAvg(prev: Double, element: Double, count: Long):


please give it a comment to make clear what the arguments are about

asurkov · 2017-11-06T23:03:43Z

src/main/kotlin/app/hashers/FactHasher.kt

    }
+
+    private fun calcIncAvg(prev: Double, element: Double, count: Long):
+        Double {


btw, I find the style when arguments are on new lines more readable than putting a return type on a new line

Nice idea, agree.

asurkov · 2017-11-06T23:43:10Z

src/main/kotlin/app/FactCodes.kt

+    val COMMITS_NUM = 9
+    // Used for number of commits per number of lines in a commit histogram.
+    // Key should be number of lines. Value number of commits.
+    val COMMITS_NUM_PER_LINE_NUM = 12


I find the name a bit confusing. Perhaps replacing 'per' to 'to' would make nicer? For example, COMMIT_COUNT_TO_LINE_COUNT. Also COMMITS -> COMMIT.

COMMIT_NUM_TO_LINE_NUM is shorter

asurkov · 2017-11-07T14:52:19Z

.gitignore

 /ultimate/ideaSDK
 /ultimate/out
 /ultimate/tmp
-src/main/resources/data/models/


asurkov · 2017-11-07T14:59:39Z

src/main/kotlin/app/hashers/CommitCrawler.kt

+            }
+            new.numLinesDeleted = new.diffs.fold(0) { total, file ->
+                total + file.getAllDeleted().size
+            }


I see the point

asurkov · 2017-11-07T15:01:13Z

src/main/kotlin/app/hashers/FactHasher.kt

    }
+
+    /**
+     * Used for incremental calculation of average of sequence.


maybe: Computes the average of a numerical sequence

asurkov · 2017-11-07T15:03:28Z

src/main/kotlin/app/FactCodes.kt

+    val COMMITS_LINE_NUM_AVG = 8
+    val COMMITS_NUM = 9
+    // A map of line numbers to commits number. Used in a commit histogram.
+    val COMMITS_NUM_TO_LINE_NUM = 12


i'm sort of concerned that commits_num has the plural form, but line_num has the single form.

asurkov · 2017-11-07T15:05:10Z

src/main/kotlin/app/FactCodes.kt

    val LINE_LONGEVITY = 3
    val LINE_LONGEVITY_REPO = 4
+    val LINE_LEN_AVG = 10
+    val LINE_NUM = 11


it'd be really nice to document all of them.

Agree, will do it

astansler added 3 commits November 6, 2017 19:36

wip: commit facts

c5c6027

wip: lines per commit

ab40081

feat: commits per lines facts

3d82b31

astansler self-assigned this Nov 6, 2017

astansler requested a review from asurkov November 6, 2017 16:50

asurkov reviewed Nov 6, 2017

View reviewed changes

astansler added 2 commits November 7, 2017 14:25

fix: pr comments

4ce2b5a

chore: add todo

c618a17

asurkov approved these changes Nov 7, 2017

View reviewed changes

astansler added 2 commits November 7, 2017 18:17

fix: const names

23cc508

chore: add comments for codes

d8214dd

astansler merged commit f7b044a into develop Nov 7, 2017

astansler deleted the dev-commit-facts branch November 7, 2017 15:50

feat: commit facts #58

feat: commit facts #58

Uh oh!

Conversation

astansler commented Nov 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astansler Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

astansler Nov 7, 2017 •

edited

Loading