[codeface] Re: [PATCH 1/2] Add option in query to return total diff size per developer

  • From: Wolfgang Mauerer <wolfgang.mauerer@xxxxxxxxxxxxxxxxx>
  • To: <codeface@xxxxxxxxxxxxx>, Mitchell Joblin <joblin.m@xxxxxxxxx>
  • Date: Thu, 26 Nov 2015 18:36:40 +0100

Hi Mitchell,

thanks for the patch -- some comments below.

Am 26/11/2015 um 17:27 schrieb Mitchell Joblin:

Signed-off-by: Mitchell Joblin <mitchell.joblin.ext@xxxxxxxxxxx>
---
codeface/R/query.r | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/codeface/R/query.r b/codeface/R/query.r
index 4b44445..68fe018 100644
--- a/codeface/R/query.r
+++ b/codeface/R/query.r
@@ -188,16 +188,19 @@ get.commits.by.ranges <- function(conf, subset=NULL,
FUN=NULL) {
}

get.commits.by.date.con <- function(con, pid, start.date, end.date,
- commit.date=TRUE, commit.count=FALSE) {
+ count.type="none", commit.date=TRUE) {

any reason to exchange the order of arguments? If you use

commit.date=TRUE, count.type="none"

instead of the above, then, for instance,
get.commits.by.date.con(con, pid, start, end, FALSE) will
deliver the same as before, whereas in your order it won't.

Not that we have so many callers, but one should sacrifice a goat
to the principle of least surprise as often as possible in
software engineering.

if (commit.date==TRUE) {
date.type <- "commitDate"
} else {
date.type <- "authorDate"
}

- if (commit.count==TRUE) {
+ if (count.type=="commit") {
query <- "SELECT author, COUNT(*) as freq"
group.by <- " GROUP BY author"
+ } else if (count.type=="loc") {
+ query <- "SELECT author, SUM(DiffSize) as freq"
+ group.by <- " GROUP BY author"
} else {
query <- "SELECT *"
group.by <- NULL

can you please add a warning (or even stop with an error) if a string
different than "none", "commit" or "loc" was given for count.type?
This way, we avoid subtleties caused by specifiying
things like count.type="comit".

Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@xxxxxxxxxxxxxxxxx>

Cheers, Wolfgang


Other related posts: