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

Potential bug in getAverageReviewInterval #31

Open
wontonst opened this issue Jan 13, 2017 · 3 comments
Open

Potential bug in getAverageReviewInterval #31

wontonst opened this issue Jan 13, 2017 · 3 comments

Comments

@wontonst
Copy link

Looking at function getAverageReviewInterval (callback)

This logic doesn't look right to me

                    reviewCreateTime = moment(mergedReviewsList[review].created);
                    reviewUpdateTime = moment(mergedReviewsList[review].updated);

                    interval = reviewUpdateTime.diff(reviewCreateTime, TIME_PERIOD_TYPE);

A commit that has been merged can still be commented on, and adding a comment changes the last updated timestamp. This shows the time between create and last updated but what we want is time between create and merge.

@wontonst
Copy link
Author

Yeah I'm convinced this logic is wrong. I've opened a stackoverflow dealing with this problem: http://stackoverflow.com/questions/41642244/how-to-get-review-time-information-from-gerrit/41642647#41642647

@tunix
Copy link
Contributor

tunix commented Jan 16, 2017

Hi @wontonst,

Thanks for sharing this with us! We'll look into it shortly. Are you willing to send a PR btw?

@wontonst
Copy link
Author

I can put it on my todo but I don't have the bandwidth at the moment. I have a working solution but it's in python.

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

No branches or pull requests

2 participants