summary.json: move sinceDate and untilDate become direct element #607 #615

emer7 · 2019-03-20T12:20:05Z

summary.json has multiple identical sinceDate and untilDate
in each repo that is analyzed, hence redundant.

This is caused by sinceDate and untilDate being instance variable
of repoConfig

Let's rectify this problem by moving sinceDate and untilDate
out of repoConfig into summaryReportJson

This PR will affect PR #596 and PR #539

emer7 · 2019-03-20T12:33:31Z

@reposense/stage1-reviewers Ready for review!

chel-seyy

The rest looks good to me

chel-seyy · 2019-03-20T13:09:12Z

src/main/java/reposense/commits/CommitInfoExtractor.java

@@ -38,8 +43,8 @@
    }

    /**
-     * Parses the {@code gitLogResult} into a list of {@code CommitInfo} and returns it.
-     */
+    * Parses the {@code gitLogResult} into a list of {@code CommitInfo} and returns it.


Spaces are missing here.

src/main/java/reposense/RepoSense.java

emer7 · 2019-03-20T18:09:34Z

@reposense/stage2-reviewers ready for review

yamidark

Changes requested

src/main/java/reposense/model/RepoConfiguration.java

yamidark · 2019-03-21T07:34:22Z

I believe the frontend portion also needs to be edited to take the sinceDate and untilDate correctly from summary.json.

emer7 · 2019-03-23T17:39:59Z

I believe the frontend portion also needs to be edited to take the sinceDate and untilDate correctly from summary.json.

No need because sinceDate and untilDate is not used so far (it is used in my other 2 PR)

emer7 · 2019-03-24T03:22:09Z

@reposense/stage2-reviewers ready for review

yamidark

1 nit, rest LGTM

yamidark · 2019-03-24T09:19:48Z

src/main/java/reposense/report/ReportGenerator.java

        InputStream is = RepoSense.class.getResourceAsStream(TEMPLATE_FILE);
        FileUtil.copyTemplate(is, outputPath);

        cloneAndAnalyzeRepos(configs, outputPath);

-        FileUtil.writeJsonFile(new SummaryReportJson(configs, generationDate), getSummaryResultPath(outputPath));
+        FileUtil.writeJsonFile(new SummaryReportJson(
+                configs, generationDate, sinceDate, untilDate),


This can be put together on the line above right (without passing the 120 characters limit)?

emer7 · 2019-03-24T14:46:54Z

@reposense/stage2-reviewers fixed last nit spotted by Jun An

eugenepeh · 2019-04-02T10:16:35Z

@eugenepeh can I know what is netlify arguments?

Its just a plain gradlew run

Because running
--since 01/03/2018 --until 15/04/2018 --config ./src/systemtest/resources on master branch
will yield the same result with running
--since 01/03/2018 --until 15/04/2018 --config ./src/systemtest/resources on 607-move-date branch
and running
--config ./src/systemtest/resources on 607-move-date branch (no date)

What config are you using and which repo is showing difference? Need to be more specific.
Are you running Delta alone? What are the files showing difference?

emer7 · 2019-04-04T07:22:27Z

What config are you using and which repo is showing difference? Need to be more specific.

I am using the config in src/systemtest/resources/repo-config.csv and src/systemtest/resources/author-config.csv
The repo that is showing difference is testrepo-Delta

Are you running Delta alone?

The repo-config has other repo beside Delta, but the repo that is showing difference is Delta given the date range of 01/03/2018 until 15/04/2018.

Why I am using that date? Because if we do not supply any date argument when running RepoSense, it will automatically analyze the repo, and by debuggin the app, the date range that I get for testrepo-Delta is 01/03/2018 until 15/04/2018. This is also why commits.json of running RepoSense without date arguments is the similar with running Reposense with date range (the only difference is the authorFinalContributionMap).

What are the files showing difference?

It is the same file you pointed out.
src/test/java/seedu/address/logic/parser/AddressBookParserTest.java

@eugenepeh

eugenepeh · 2019-04-04T11:23:40Z

@emer7

The questions I asked above is things that you should include and investigate when reporting a bug / error.

Main problem is still that the new changes doesn't seems to be correct. If you think that there is an underlying problem, you will need to investigate and report it as an issue.

…607-move-date

emer7 · 2019-04-08T16:23:26Z

@eugenepeh
This is what I get after investigating:

Running RepoSense without any date argument, upon reaching

// CommitInfoExtractor.java#extractCommitInfos:33
String gitLogResult = GitLog.get(config, author);

It will get the git log, but it seems to get it on arbitrary date range. In the case of testrepo-delta, it gets all the commits from 01/03/2018 to 15/04/2018.
But when it reaches CommitInfoExtractor.java#extractFileInfos, it will get all the file info regardless of date range if it is not provided.

This behavior is what causing running
--config ./src/systemtest/resources --since 01/03/2018 --until 15/04/2018 on master branch
and
--config ./src/systemtest/resources on master branch
to have almost the same commits.json but different authorship.json I believe.
Almost the same as in the commits list are the same in commits.json, but there is slight difference in contribution, etc. (will investigate further).

So is this an intended design or a bug?

eugenepeh · 2019-04-09T15:52:00Z

But when it reaches CommitInfoExtractor.java#extractFileInfos, it will get all the file info regardless of date range if it is not provided.

It is meant to retrieve all the affected files in the given date range.

So is this an intended design or a bug?

If this only cause an issue when a date range is issued, then most likely it is a bug.

Almost the same as in the commits list are the same in commits.json, but there is slight difference in contribution, etc. (will investigate further).

Yes, please do investigate further. It will help you understand the back end architecture better if you haven't already.

…607-move-date

Because it will interfere with authorship report

emer7 · 2019-04-10T14:05:10Z

@eugenepeh fixed and worked already. No bug

eugenepeh

👍 minor nits

eugenepeh · 2019-04-10T14:32:30Z

src/main/java/reposense/report/ReportGenerator.java

@@ -227,4 +238,16 @@ private static String getIndividualAuthorshipPath(String repoReportDirectory) {
    private static String getIndividualCommitsPath(String repoReportDirectory) {
        return repoReportDirectory + "/commits.json";
    }
+
+    public static void setEarliestSinceDate(Date newEarliestSinceDate) {
+        if (earliestSinceDate == null || earliestSinceDate.after(newEarliestSinceDate)) {


I think normally when people look for the earliest date/object, they usually compare who comes before, instead of after, in multiple comparison context. To standardize with that convention, let's change it to use before instead

@eugenepeh done

yamidark · 2019-04-11T07:28:24Z

@emer7 Do resolve the conflicts

…607-move-date

emer7 · 2019-04-12T16:03:05Z

@yamidark resolved

yamidark · 2019-04-16T09:56:37Z

Slight nits to proposed message:

summary.json has multiple identical sinceDate and untilDate
in each repo that is analyzed, which is redundant.

This is caused by sinceDate and untilDate being instance variable
of RepoConfiguration.

Let's rectify this problem by moving sinceDate and untilDate
out of RepoConfiguration into SummaryReportJson.

Move out sinceDate and untilDate out of repoConfig

ef628bb

emer7 added the s.ToReview label Mar 20, 2019

emer7 requested a review from a team March 20, 2019 12:33

Merge branch 'master' into 607-move-date

4b92021

chel-seyy approved these changes Mar 20, 2019

View reviewed changes

fzdy1914 reviewed Mar 20, 2019

View reviewed changes

src/main/java/reposense/RepoSense.java Show resolved Hide resolved

emer7 requested a review from a team March 20, 2019 18:06

Spacing

11dbeee

emer7 closed this Mar 20, 2019

emer7 reopened this Mar 20, 2019

yamidark suggested changes Mar 21, 2019

View reviewed changes

src/main/java/reposense/model/RepoConfiguration.java Show resolved Hide resolved

yamidark added s.Ongoing and removed s.ToReview labels Mar 21, 2019

emer7 added 4 commits March 23, 2019 02:24

Undo changes and set transient variable

344cd72

Merge branch 'master' into 607-move-date

17526aa

Fix style

cb5c728

Remove wrong line

5183da7

emer7 requested a review from a team March 23, 2019 17:40

emer7 added s.ToReview and removed s.Ongoing labels Mar 23, 2019

yamidark approved these changes Mar 24, 2019

View reviewed changes

Fix style

d0bfeb5

emer7 requested a review from a team March 24, 2019 14:37

yamidark changed the title ~~Move sinceDate and untilDate in summary.json to be direct element of summary.json #607~~ summary.json: move sinceDate and untilDate become direct element #607 Mar 25, 2019

emer7 unassigned yamidark and eugenepeh Apr 4, 2019

Merge branch 'master' of https://github.com/reposense/RepoSense into …

770e5c7

…607-move-date

emer7 added 4 commits April 10, 2019 16:33

Merge branch 'master' of https://github.com/reposense/RepoSense into …

71f2d88

…607-move-date

Change implementation to not using config

89a6b91

Because it will interfere with authorship report

Fix systemtest

c1f6ac7

Fix codacy

9d4dcbf

emer7 requested a review from a team April 10, 2019 14:04

eugenepeh reviewed Apr 10, 2019

View reviewed changes

Flip before/after

911f657

emer7 requested a review from a team April 11, 2019 06:05

yamidark approved these changes Apr 11, 2019

View reviewed changes

yamidark requested a review from eugenepeh April 11, 2019 06:40

Merge branch 'master' into 607-move-date

b4e091d

Merge branch 'master' of https://github.com/reposense/RepoSense into …

3fd10ee

…607-move-date

emer7 requested a review from a team April 14, 2019 07:39

eugenepeh approved these changes Apr 15, 2019

View reviewed changes

yamidark added s.ToMerge and removed s.ToReview labels Apr 15, 2019

yamidark approved these changes Apr 15, 2019

View reviewed changes

yamidark merged commit 8f385ab into reposense:master Apr 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

summary.json: move sinceDate and untilDate become direct element #607 #615

summary.json: move sinceDate and untilDate become direct element #607 #615

emer7 commented Mar 20, 2019 •

edited

emer7 commented Mar 20, 2019

chel-seyy left a comment

chel-seyy Mar 20, 2019

emer7 commented Mar 20, 2019

yamidark left a comment

yamidark commented Mar 21, 2019

emer7 commented Mar 23, 2019

emer7 commented Mar 24, 2019

yamidark left a comment

yamidark Mar 24, 2019

emer7 commented Mar 24, 2019

eugenepeh commented Apr 2, 2019

emer7 commented Apr 4, 2019 •

edited

eugenepeh commented Apr 4, 2019

emer7 commented Apr 8, 2019 •

edited

eugenepeh commented Apr 9, 2019

emer7 commented Apr 10, 2019

eugenepeh left a comment

eugenepeh Apr 10, 2019

emer7 Apr 10, 2019

yamidark commented Apr 11, 2019

emer7 commented Apr 12, 2019

yamidark commented Apr 16, 2019

summary.json: move sinceDate and untilDate become direct element #607 #615

summary.json: move sinceDate and untilDate become direct element #607 #615

Conversation

emer7 commented Mar 20, 2019 • edited

emer7 commented Mar 20, 2019

chel-seyy left a comment

Choose a reason for hiding this comment

chel-seyy Mar 20, 2019

Choose a reason for hiding this comment

emer7 commented Mar 20, 2019

yamidark left a comment

Choose a reason for hiding this comment

yamidark commented Mar 21, 2019

emer7 commented Mar 23, 2019

emer7 commented Mar 24, 2019

yamidark left a comment

Choose a reason for hiding this comment

yamidark Mar 24, 2019

Choose a reason for hiding this comment

emer7 commented Mar 24, 2019

eugenepeh commented Apr 2, 2019

emer7 commented Apr 4, 2019 • edited

eugenepeh commented Apr 4, 2019

emer7 commented Apr 8, 2019 • edited

eugenepeh commented Apr 9, 2019

emer7 commented Apr 10, 2019

eugenepeh left a comment

Choose a reason for hiding this comment

eugenepeh Apr 10, 2019

Choose a reason for hiding this comment

emer7 Apr 10, 2019

Choose a reason for hiding this comment

yamidark commented Apr 11, 2019

emer7 commented Apr 12, 2019

yamidark commented Apr 16, 2019

emer7 commented Mar 20, 2019 •

edited

emer7 commented Apr 4, 2019 •

edited

emer7 commented Apr 8, 2019 •

edited