Rephrase 'file not found' logs shown during first run #139

[LimJunxue]

Fixes #139 Result:

[canihasreview[bot]]

Click here to submit a new iteration when this PR is ready for review.

See this repository's contribution guide for more information.

[canihasreview[bot]]

v1

@LimJunxue submitted v1 for review.

• [v1 1/1] Add logs for creation of default files

(:books: Archive)

Checkout this PR version locally

```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/152/1/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.

[damithc]

@LimJunxue the log messages that caused the initial confusion are still there. Can we replace (rather than add onto) those messages with the new messages?

[LimJunxue]

@damithc hmm the Json file ... not found. log comes from JsonUtil#readJsonFile which does not create the new files though. Will it be better if we change the Config file not found. Will be starting with a new default config file. lines to Config file not yet **created**. Will be starting with a new default config file. while still keeping the JsonUtil log?

[damithc]

@damithc hmm the Json file ... not found. log comes from JsonUtil#readJsonFile which does not create the new files though.

I see. We can try lowering the log level of the initial log message e.g., logger.fine so that it appears only for developers specifically running the app at a lower lower log level (assuming the default log level is info). If that doesn't work, we can also consider removing it altogether. What do you think?

[LimJunxue]

The changing of log level issue is still unfixed which might cause some confusion for developers. So I think removing the log is fine too because it is not needed when the added logs covers all uses of JsonUtil#readJsonFile in this project.

[damithc]

The changing of log level issue is still unfixed which might cause some confusion for developers.

Are you sure? When I changed the line as follows, it doesn't appear in the console anymore.

logger.fine("Json file " + filePath + " not found");

But you are right in that developers might be confused if they don't know that fine level log messages don't appear by default.

Another option is to report when the file is found, rather than not found:

        if (!Files.exists(filePath)) {
            return Optional.empty();
        }
        logger.info("Json file " + filePath + " found");

[LimJunxue]

The changing of log level issue is still unfixed which might cause some confusion for developers.

This was referring to #126

The result will be this, is this ok?

[damithc]

The result will be this, is this ok?

@LimJunxue This one is OK. The more interesting one is the case of the file not being found. That's the one that confused students.

[codecov-commenter]

Codecov Report

Merging #152 (d560881) into master (412b5ed) will decrease coverage by 0.23%. The diff coverage is 20.00%.

@@             Coverage Diff              @@
##             master     #152      +/-   ##
============================================
- Coverage     72.15%   71.92%   -0.24%     
  Complexity      399      399              
============================================
  Files            70       70              
  Lines          1232     1236       +4     
  Branches        125      127       +2     
============================================
  Hits            889      889              
- Misses          311      315       +4     
  Partials         32       32              

Impacted Files	Coverage Δ
src/main/java/seedu/address/MainApp.java	0.00% <0.00%> (ø)
...main/java/seedu/address/commons/util/JsonUtil.java	94.11% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[canihasreview[bot]]

v2

@LimJunxue submitted v2 for review.

• [v2 1/2] Add logs for creation of default files
• [v2 2/2] Change file not found log to file found

(:books: Archive) (:chart_with_upwards_trend: Interdiff between v1 and v2) (:chart_with_upwards_trend: Range-Diff between v1 and v2)

Checkout this PR version locally

```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/152/2/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.

[damithc]

@se-edu/tech-team-level1 for your review ...

[damithc]

Sorry, it took a while for me to come back to this PR. I'm wondering if including the target filename in the log message is better (rather than the current Prefs file not found. ..., Preferences file prefs.json not found. ...)

[damithc]

Closing in favor of #170